Skip to content

Add unit and regression tests for conformance tests #531

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 34 commits into from
Mar 22, 2024
Merged

Add unit and regression tests for conformance tests #531

merged 34 commits into from
Mar 22, 2024

Conversation

martinmo
Copy link
Member

@martinmo martinmo commented Mar 20, 2024 •
edited
Loading

This PR adds basic unit and regression test infrastructure for our conformance tests in ./Tests using the pytest tool.

Included are

  • a new GitHub workflow that runs pytest tests and blocks merging if tests fail,
  • unit and regression tests for the most important bits of the version policy and flavor naming conformance tests,
  • a section in Tests/README.md explaining how to use the unit tests,
  • bug fixes for bugs found by the new tests,
  • and last but not least some refactorings I couldn't resist to perform already.

Resolves #476.

@martinmo martinmo added the SCS-VP10 Related to tender lot SCS-VP10 label Mar 20, 2024
@martinmo martinmo self-assigned this Mar 20, 2024
martinmo added 17 commits March 20, 2024 15:41
@martinmo
Add a first batch of unit tests for k8s_version_policy
5b454ff 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Fix bugs found with the new unit tests
477afe3 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Add more tests
d4d8ddd 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Update Tests/README.md with infos on unit testing
a79182a 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Bump deprecated GitHub actions
4fd7a46 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Explicitly specify the Python minor version for linting
30829ab 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Add GitHub workflow for pytest unit tests
79747ef 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Fix installation of requirements in GitHub workflow
281503a 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Improve documentation again
cf4682b 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Use a dedicated job name for pytest tests
cbd801b 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Improve instructions and fix wrong heading level
5883986 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Rename check-yaml.py to make it importable
7b995e8 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Make check_yaml easier to test with pytest
2f39e4f 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Add regression tests for check_yaml
9b27838 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Fix intra document link
557a804 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Modernize code with pathlib.Path
5b82730 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Use better variable names to avoid confusion
5ca8589 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo martinmo force-pushed the issue/476-qa-for-conformance-tests branch from 8653e22 to 5ca8589 Compare March 20, 2024 14:41
@martinmo martinmo force-pushed the issue/476-qa-for-conformance-tests branch from fbca1fa to 15948b5 Compare March 21, 2024 07:43
@mbuechse
Copy link
Contributor

This seems to be progressing nicely. Will you remove the Draft state soon?

@martinmo martinmo marked this pull request as ready for review March 22, 2024 10:05
@martinmo martinmo requested a review from mbuechse March 22, 2024 10:05
@martinmo
Copy link
Member Author

This seems to be progressing nicely. Will you remove the Draft state soon?

Thank you! It is ready for review now. I've updated the PR description accordingly.

I took a bit longer because of layer 8 problem on my part – I spent a lot of time debuggig an issue which turned out to be caused by a tiny typo 🙈

mbuechse
mbuechse approved these changes Mar 22, 2024
View reviewed changes
Copy link
Contributor

@mbuechse mbuechse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I have a few minor remarks, as usual... but that is not meant to diminish anything!

martinmo and others added 3 commits March 22, 2024 13:17
@martinmo
Improve the READMEs as suggested by @mbuechse
b05e7cd 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Improve code based on suggestions by @mbuechse
5df9178 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo @mbuechse
Improve affected version check as suggested by @mbuechse
91785ad 
Co-authored-by: Matthias Büchse <[email protected]>
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo martinmo requested a review from mbuechse March 22, 2024 12:33
@martinmo
Copy link
Member Author

The markdown linter complains, I'm on it...

martinmo added 2 commits March 22, 2024 13:36
@martinmo
Fix markdown issues
f58d2d4 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo
Add missing language to fenced code block
bd27d76 
Signed-off-by: Martin Morgenstern <[email protected]>
@martinmo martinmo marked this pull request as draft March 22, 2024 13:12
@martinmo
Copy link
Member Author

Converted to draft again to block merging, I found another bug when testing with minikube, unfortunately...

CRITICAL: '<=' not supported between instances of 'NoneType' and 'K8sVersion'

I'm currently investigating.

@martinmo martinmo marked this pull request as ready for review March 22, 2024 13:58
@martinmo
Copy link
Member Author

The issue is now resolved. TL;DR: it was bad handling of the case when the lower version of the affected version range was not specified. (The bigger picture is that the VersionRange class is not the best abstraction to cover all use cases – but because this PR is scoped to the tests, I don't want to change it completely. This will be done in #526.)

@mbuechse
Copy link
Contributor

You are GO for launch!
https://www.youtube.com/watch?v=lMtWWls4oas

@martinmo martinmo merged commit 2cf8efd into main Mar 22, 2024
@martinmo martinmo deleted the issue/476-qa-for-conformance-tests branch March 22, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbuechse mbuechse Awaiting requested review from mbuechse

Assignees

@martinmo martinmo

Labels
SCS-VP10 Related to tender lot SCS-VP10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression and unit tests for the standards testsuite
2 participants
@martinmo @mbuechse