-
Notifications
You must be signed in to change notification settings - Fork 780
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
test: add some audit tests #2489
Conversation
Signed-off-by: Sertac Ozercan <[email protected]>
Codecov ReportBase: 53.45% // Head: 53.73% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2489 +/- ##
==========================================
+ Coverage 53.45% 53.73% +0.28%
==========================================
Files 115 115
Lines 10196 10196
==========================================
+ Hits 5450 5479 +29
+ Misses 4329 4299 -30
- Partials 417 418 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: Sertac Ozercan <[email protected]>
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.
Thanks for starting to add tests!
We probably want to test the actual auditing logic at some point, but as long as we're not closing the bug.
Only blocking concerns are nits about using t.Run()
} | ||
} | ||
|
||
func Test_getFilesFromDir(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.
We probably want to test the round-tripping of writing/reading list results in order to catch inconsistencies (since the reading/writing logic is necessarily coupled).
Doing this would probably require some refactoring, which I could see being part of a larger effort later.
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.
are we okay to do this as a follow-up?
Signed-off-by: Sertac Ozercan <[email protected]>
Yes, these are just quick initial tests that don't require dealing with fake/mock clients. This is not closing the issue. |
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
Signed-off-by: Sertac Ozercan [email protected]
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):related #2453
Special notes for your reviewer: