-
Notifications
You must be signed in to change notification settings - Fork 373
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
pkg/testutil: add in-memory Plugin builder #283
pkg/testutil: add in-memory Plugin builder #283
Conversation
c566403
to
2c2ff85
Compare
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #283 +/- ##
==========================================
- Coverage 55.07% 54.91% -0.16%
==========================================
Files 19 19
Lines 877 874 -3
==========================================
- Hits 483 480 -3
Misses 341 341
Partials 53 53
Continue to review full report at Codecov.
|
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 reads very nicely. Well done!
There are some left-over Plugin usages in util_test.go
and platform_test.go
which could be cleaned up. Did you leave those out intentionally?
Yeah it seems like I forgot. |
So looks like platform.go can't use this utility package, because it uses a cyclic import when used in test case I'm planning to move this method to |
Introducing builder-pattern methods to create in-memory Plugin/Platform objects that has good defaults, but can be overwritten for test purposes. Readability of the test files (e.g. validation/*_test.go) now speak for themselves. The testutil.NewPlugin() returns a _valid_ index.Plugin instance. Any tests expected to fail can modify its values with the builder pattern. (ditto for testutil.NewPlatform()). - had to move validation code to pkg/index/validation due to a cyclic import situation that occurred. This should be fine. - renamed some test cases for ease of readability - I've thought about adding code validation for **_test.go to not to have any index.Plugin{ or index.Platform{ strings. But this might be an overkill atm. Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
2c2ff85
to
ffea965
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmetb, corneliusweig The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Introducing builder-pattern methods to create in-memory Plugin/Platform objects
that has good defaults, but can be overwritten for test purposes. Readability
of the test files (e.g. validation/*_test.go) now speak for themselves.
The testutil.NewPlugin() returns a valid index.Plugin instance. Any tests
expected to fail can modify its values with the builder pattern. (ditto for
testutil.NewPlatform()).
import situation that occurred. This should be fine.
index.Plugin{
orindex.Platform{
strings. But this might be an overkill.Fixes #270. Beware of merge conflicts that will raise due to in-flight PRs.