Conversation
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
| deppy.Identifier("fake-catalog-test-package-test-package.v1.0.1"), | ||
| ), | ||
| ), | ||
| ID: "test-package package uniqueness", |
There was a problem hiding this comment.
This is not the best example, but I'm trying to find a nicier way to do these comparisons.
E.g. in #498 we do this:
gocmpopts := []cmp.Option{
cmpopts.IgnoreUnexported(sync.RWMutex{}),
cmp.AllowUnexported(
sync.RWMutex{},
olmvariables.BundleVariable{},
input.SimpleVariable{},
constraint.DependencyConstraint{},
catalogmetadata.Bundle{},
),
cmpopts.IgnoreFields(
// Do not compare a func field
catalogmetadata.PackageRequired{}, "SemverRange",
),
}
require.Empty(t, cmp.Diff(bundles, expectedVariables, gocmpopts...))There was a problem hiding this comment.
Haven't had enough time to review all of it - but you likely do not want to compare internals of sync.RWMutex - perhaps even of the others. For each, ask if it's possible or valid for the third-party package to change the implementation detail of unexported fields and if that would be a bona-fide concern for the caller here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #505 +/- ##
==========================================
+ Coverage 83.92% 83.97% +0.05%
==========================================
Files 23 23
Lines 877 880 +3
==========================================
+ Hits 736 739 +3
Misses 96 96
Partials 45 45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Closing for now, but I'm hoping to come back to it at some point. |
To do:
go-cmpinvariablesources/installed_package_test.go.Description
Experiment to make testing a bit easier.
Reviewer Checklist