-
-
Notifications
You must be signed in to change notification settings - Fork 117
Refactor unit tests to support unittests v0.4.4 #417
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
Conversation
cognifloyd
left a comment
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.
Would you please update the quintush/helm-unittest references in tests/README.md as well?
And I have one non-blocking nitpick in .github/workflows/unit.yaml.
Everything else looks great! Thank you for picking this up!
| # as the de-facto "best" fork has changed several times over the years. | ||
| run: | | ||
| helm plugin install https://github.com/quintush/helm-unittest --version v0.2.11 | ||
| helm plugin install https://github.com/helm-unittest/helm-unittest.git --version v0.4.4 |
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.
Is .git required?
| helm plugin install https://github.com/helm-unittest/helm-unittest.git --version v0.4.4 | |
| helm plugin install https://github.com/helm-unittest/helm-unittest --version v0.4.4 |
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.
.git doesn't seem to be required (I tested and it works w/o) - but the unittest documentation https://github.com/helm-unittest/helm-unittest/tree/main?tab=readme-ov-file#install has .git in the install command.
Unsure if we want to remove it, or leave it as is since thats the documented install FQDN.
Documentation updated |
Implements #414
Updated our
tests/unitto support newer versions ofunittests- for now bumping tov0.4.4asv0.5.0has a bug that impacts us (see helm-unittest/helm-unittest#329)*Changes required:
v0.3.0ofunittestsaddsjsonPathwhich required updating ourpathreferences to use ajqstyle compatible syntaxisNullchanged behavior, such that if the path exists but has an "empty" value - the assertion now fails, i.e. the belowk8sdefinition doesn't pass (when it did before), since having emptykeysdoesn't do anything functionally when applied tok8s, and in my opinion atleast, looks displeasing/confusing - I fixed up thehelmtemplates to not produce emptyimagePullSecretsif nothing is being set - fixing the test again.v0.3.1(includingisNull, which seems to have also been when the aforementioned behavior change was introduced), other that the above issue, nothing else behaviorally seems to have changed and the old names still work - however to ensure we're future proofed in-case they are removed (in the docs atleastisNullandisNotNullare mentioned to be "deprecated", I've update the following assertions:isNotNullwithexistsisNullwithnotExistsisEmptywithisNullOrEmptyisNotEmptywithisNotNullOrEmptyWith those changes, our tests now work on
v0.4.4(and should work onv0.5.xonce the previously mentioned bug is fixed)The pipeline also has been updated to use
v0.4.4v0.5.0so once we have a fixedv0.5.xversion we'll be able to bump to that with hopefully no further changes.