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

Add tests for current behaviour of --monitor-snapshots #729

Open
wants to merge 51 commits into
base: master
Choose a base branch
from

Conversation

Hooloovoo
Copy link

Suggested commit message for a squash and merge:

  • Create tests for the current behaviour of --monitor-snapshots.
  • Fixed tests/common/lib.sh, which was not clearing the snapshot cache in between tests because of a missing "/"
  • Add tests/README.md to give instructions to new contributors on how to run the tests.

Explanation:
I have some changes I would like to commit in another branch that adds JSON metrics output (see https://github.com/Hooloovoo/sanoid/tree/add_metrics_json and #675 ), but it involves changes to the monitor-snapshots code and I wanted to ensure I was not changing any existing behaviour, so I have added tests for the current behaviour here. This is also why I chose to write the tests in Python (in addition to that being my preferred language), as I intend to extend these tests in my JSON branch to test that the new sanoid JSON output can be correctly parsed by the Python JSON library, which is how I want to consume them to create Prometheus metrics: https://gitlab.com/aaron-w/sanoid_prometheus

I have checked that the three tests all pass (I would prefer to rename these test folders so that the quickest ones are first and the very slow one is last, so that developers can get more immediate feedback, but I did not do this in case it might break something you do in testing before release etc).

@Hooloovoo Hooloovoo mentioned this pull request Aug 16, 2022
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.

1 participant