Skip to content
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

tests: add more tests #1002

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lorenzofelletti
Copy link
Contributor

In light of the recent issue with the API server availability rules, which was hard to catch by simply looking at the git diff, I think that adding more thorough testing is due to make repeating such a mistake harder in the future.

In this PR I'm adding more tests around the API server rules, and I'm also moving the tests to a dedicated folder, allowing to break the giant tests.yam file in multiple, more readable ones. The Makefile is updated accordingly, so to run all test files in the tests folder on make test.

@skl
Copy link
Collaborator

skl commented Dec 17, 2024

Thank you for doing this, @lorenzofelletti! I was thinking the same.

One thing to watch out for - there's a few dependabot PRs open which seem to depend on go.mod being bumped to 1.23, as well as the prometheus version being bumped from 0.53 to 0.300, for example:

Note that the prometheus version bump appears to change the behaviour of the tests (understandable given how out of date the deps were).

I can fix these on #994 but you'll need to make sure to merge/rebase latest master branch, just wanted to give you a heads up in case your test behaviour is affected.

@lorenzofelletti
Copy link
Contributor Author

Hi @skl, thanks for letting me know. I'll rebase/merge master to keep this in sync.

@skl
Copy link
Collaborator

skl commented Dec 19, 2024

@lorenzofelletti deps updated including prometheus, fixed tests to match latest behaviour in a2e2d9a. Please pull latest master and apologies for the conflicts!

values: '0 0 0 1 1 1'

promql_expr_test:
- eval_time: 5m
Copy link
Collaborator

Choose a reason for hiding this comment

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

btw I just found out that you can name tests, this is really useful for identifying which test has failed and can indicate the intention of the test, in case you'd like to use the name property (see docs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good finding! Will start using it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants