-
Notifications
You must be signed in to change notification settings - Fork 65
dlog v1 finalization: unit-tests coverage above 80% - token #1035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
df79064
to
2ff7bb6
Compare
d2fbba9
to
e989579
Compare
{ | ||
name: "publicParams cannot be nil", | ||
init: func() (logging.Logger, *setup.PublicParams, token2.IdentityDeserializer, error) { | ||
return nil, nil, nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to return the logger? we could use some default or nil value always, or are we testing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo comments
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that using matrices here makes the tests more complex. The amount of re-usable code is negligible, so I would inline the tests, e.g.
func Test_Success(t *testing.T) {
RegisterTestingT(t)
id := &mock.IdentityDeserializer{}
id.RecipientsReturns([]driver.Identity{driver.Identity("alice")}, nil)
ts, err := token2.NewTokensService(nil, pp, id)
Expect(err).ToNot(HaveOccurred())
tok := &token2.Token{}
raw, err := tok.Serialize()
Expect(err).ToNot(HaveOccurred())
identities, err := ts.Recipients(raw)
Expect(err).ToNot(HaveOccurred())
Expect(identities).To(ConsistOf(driver.Identity("Alice"))
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way also your errors can become more descriptive, e.g.
Expect(err).To(ContainSubstring(...))
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
No description provided.