Improve TestListReleases for /pkg/agent #1369
Conversation
- Check the content of returned listed releases (not just size comparison). - Complete information about returned apps in test cases. - Add test case for release with same name, but different versions and namespaces.
| } | ||
|
|
||
| for _, app := range apps { | ||
| rs := releaseStub{app.ReleaseName, app.Namespace, strtoi(app.Version)} |
There was a problem hiding this comment.
So I think this is comparing a manually constructed releaseStub made from the output AppOverview, with another manually constructed releaseStub created from the expected AppOverview. Why not just compare the output AppOverview with the expected AppOverview directly? (bonus that they then have the same types).
Also, not sure if it could have been used here, but the cmp from the go-cmp module is quite nice for comparisons... you might also be able to do something like:
import "github.com/google/go-cmp/cmp"
...
if !cmp.Equal(tc.expectedApps, apps) {
t.Errorf(cmp.Diff(tc.expectedApps, apps))
}
There was a problem hiding this comment.
Absolutely, comparing AppOverView's makes more sense.
I'll take a look at go-cmp, but I have a gut feeling that additional fields need to be filled in the expected AppOverview array.
I'll create a new PR since this one is merged & closed.
There was a problem hiding this comment.
I tried that while reviewing this PR and it doesn't work (that's why I assumed it was doing the comparison manually). There are still some fields that are not in the expected release (like the status of the release) and also the order in the slices changes sometimes so the comparison fails.
There was a problem hiding this comment.
I was able to fix it by using the library @absoludity mentioned in PR #1379.
It was done by mapping a unique id (presumably no two releases can have the same name in the same namespaces in the expected results) to a pointer to the expected AppOverview.
Then, by generating a unique id for the attained result, looking up it in the mapping, and then and only then, perform deep equality check on both objects.
There was a problem hiding this comment.
Looks like it got even simpler than that too. Great, thanks @latiif
Description of the change
Build and expand on the existing test of
TestListReleasesin/pkg/agent, by ensuring the equivalence between what is expected and the returned values ofListReleases.The current test checks only if
|G| = |W|whereWis the expected result andGis the result ofListReleases.Benefits
By populating the expected results of the test cases with
namespaceandversion, better distinction between releases is achieved.This PR adds an additional constraint, namely
∀ g ∊ G : g ∊ W, which together with the previous one i.e.|G| = |W|, ensures thatG = W.The new test case tests the scenario where
ListReleasesreturns two apps with same name but different namespaces and versions.Applicable issues