Fix functionality (audit, loglevel) tests to actually wait for changes #233
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the functionality tests that tested the log level changes were not waiting for rollouts to happen, so they were effectively not testing that the logLevel and audit flags were properly set on the deployment. Recent releases have changed that timing so these tests have started to fail. This changes the test such that they now wait for controller to process each of the changes (or not process each of the changes, depending on the test case). It does this by adding an additional deployment annotation for each test case, so the test case knows that the controller has processed the test case and it can check the values. This probably needs a more complete refactor at some point, but this at least makes the test dependable. The audit config tests don't work perfectly either, but that is harder to solve because the functionality they are testing is currently kind of broken, i.e. you can't currently deconfigure audit, so you can't ever reset the test to a "clean" state to try the next test case. For now I made them work by having them sequentially add the args without resetting. Signed-off-by: John Kyros <[email protected]>
jkyros
force-pushed
the
make-loglevel-test-work
branch
from
68acfb6 to
c926be8
Compare
jkyros
changed the title
jkyros
changed the title
|
/lgtm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This functionality test has been failing since we did our last bump.
It was honestly never really testing anything because it never waited for the deployments, we were mostly getting lucky and not checking errors. See this run for an example of the old "tests nothing and succeeds" behavior: https://github.com/kedacore/keda-olm-operator/actions/runs/8209089746/job/22472571935
This change is basically:
testCaseannotation to the deployment containing the test case name when we attempt an attribute changeI'm not in love with this fix, but I think it should be dependable until we can make some deliberate decisions/refactors.
NOTE: The audit config tests don't work right either, but I only wiggled them a little bit, because they are harder to solve because the functionality they are testing is currently broken, i.e. you can't currently deconfigure audit, so you can't ever reset the operator to a "clean" state to try the next test case. (That has to do with how we calculate our manifestival changes from the previous manifest and not the base manifest, and how we only transform if our values are non-empty [1] [2] ). We can and probably should fix that later.
Checklist
Fixes #