-
Notifications
You must be signed in to change notification settings - Fork 151
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
Cleanup kustomize manifests rendering action #1353
Cleanup kustomize manifests rendering action #1353
Conversation
switch i { | ||
case 0: | ||
g.Expect(rc).Should(BeNumerically("==", 1)) | ||
case 1: | ||
g.Expect(rc).Should(BeNumerically("==", 2)) | ||
case 2: | ||
g.Expect(rc).Should(BeNumerically("==", 2)) | ||
} |
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.
not sure i fully understand this case:
so we have 3 replica of dashboard,
for the 1st one, gauge should set be 1
for the 2nd one, then gauge to 2
and the last one, to 3.
or i mistunderstood this
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.
no, this is not about replicas, this is about testing the cache so:
- 1st iteration, the counter should be 1, because nothing did exist before
- 2nd iteration, it should be 2, because the
Generation
field of the live object has changed - 3rd iteration, it should still 3, because nothing has changed so no rendering should have been performed and values are taken from the cache
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.
ok i see, so the test is to generate a 3 times-loop to minic :
i = 0, first time creation, no cache, increase in guage
i= 1, render when generateion already as 1, get from cache, increase in gauge
i= 2, render but with same generation value, no increase in gauge
no need more, till be same as i=2
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.
So, case 1,2:
that you do not spend effort to compare symbol by symbol equal branches?
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.
in 1,2 is where the action return the previously rendered resources because the source of customization has not changed
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature-operator-refactor #1353 +/- ##
============================================================
Coverage ? 24.45%
============================================================
Files ? 53
Lines ? 4294
Branches ? 0
============================================================
Hits ? 1050
Misses ? 3113
Partials ? 131 ☔ View full report in Codecov by Sentry. |
This commit is meant to cleanup the kustomize manifest redering by: - removing a useless parameter related to the caching mechanism - moving the code to the kustomize package to accomodate other rendering engines - introduce metrics tro track how many resources have been rendered and to perform blackbox testing against the action
bc1410e
to
5ccb9c4
Compare
switch i { | ||
case 0: | ||
g.Expect(rc).Should(BeNumerically("==", 1)) | ||
case 1: | ||
g.Expect(rc).Should(BeNumerically("==", 2)) | ||
case 2: | ||
g.Expect(rc).Should(BeNumerically("==", 2)) | ||
} |
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.
ok i see, so the test is to generate a 3 times-loop to minic :
i = 0, first time creation, no cache, increase in guage
i= 1, render when generateion already as 1, get from cache, increase in gauge
i= 2, render but with same generation value, no increase in gauge
no need more, till be same as i=2
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zdtsw 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 |
f941791
into
opendatahub-io:feature-operator-refactor
This commit is meant to cleanup the kustomize manifest redering by:
engines
to perform blackbox testing against the action
Description
How Has This Been Tested?
Screenshot or short clip
Merge criteria