Simplify nstemplatetiers.GenerateTiers#486
Simplify nstemplatetiers.GenerateTiers#486metlos merged 2 commits intocodeready-toolchain:masterfrom
Conversation
…eate/update in the EnsureObject func. This is in preparation for using the SSA client that doesn't do that either.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #486 +/- ##
==========================================
- Coverage 78.70% 78.62% -0.09%
==========================================
Files 52 52
Lines 2648 2638 -10
==========================================
- Hits 2084 2074 -10
Misses 502 502
Partials 62 62
🚀 New features to boost your workflow:
|
|
/retest |
| // check that the tier templates are unchanged | ||
| for _, tierTmpl := range tierTmpls.Items { | ||
| assert.Equal(t, int64(1), tierTmpl.Generation) // unchanged | ||
| assert.True(t, reflect.DeepEqual(origTemplates[tierTmpl.Name].Spec, tierTmpl.Spec)) |
There was a problem hiding this comment.
You could do
| assert.True(t, reflect.DeepEqual(origTemplates[tierTmpl.Name].Spec, tierTmpl.Spec)) | |
| assert.Equal(t, origTemplates[tierTmpl.Name].Spec, tierTmpl.Spec) |
instead, right?
Or why would the generation change here? The tier is not going to be updated for the same template, right? Basically why you need to change the test here at all?
There was a problem hiding this comment.
Huh, this led me down a rabbit hole. I changed this because the generation was being changed during a single patch call, so I was suspecting our test client doing something weird.
And it indeed was weird. The actual "error" because of which the generation was changed to 2 was caused by our implementation of test.Patch (the default patching behavior on top of the fake client).
We want determine whether to increase the generation there by reading the object from the cluster, dry-running the apply on a copy of the object and then comparing the two.
Now, the "original" object comes directly from the filesystem and contains the template as "raw extension", that is read as just a []byte. When you save it to the cluster, the raw extension gets serialized for some reason.
Now, because some of the templates explicitly declared a zero value of some properties (namely ClusterResourceQuota.spec.selector.annotation) the representation didn't match what came deserialized back from the cluster (which didn't contain the zero value anymore, understandably).
To capture this in the future, I added a comment and a second check on the same place - one check is using my method of deep equal and the other is the original generation change check. If we see just one of them failing, we know where to look the next time.
Thanks for pointing this out. Took me a bit to figure this out but the test is now better.
metlos
force-pushed
the
ssa-bundled-tiers
branch
3 times, most recently
from
11cfb37 to
45b3fec
Compare
…sible test problems with the test templates
|
3 participants
Simplify nstemplatetiers.GenerateTiers to not distiniguish between create/update in the EnsureObject func. This is in preparation for using the SSA client that doesn't do that either.
Related PRs:
JIRA: https://issues.redhat.com/browse/SANDBOX-1339