-
Notifications
You must be signed in to change notification settings - Fork 3
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
test fixtures #55
Merged
test fixtures #55
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
Most of the times, it's 200, so the default value simplifies usage in actual tests.
This fixture itself uses the 'use_old_replica_state' fixture, so that it's no longer needed to use it explicitly in test functions.
Instead of defining the CliRunner value in each test, we use a fixture. The CliRunner is also configured with stdout and stderr separated because mixing them will pose problem if we use stderr for other purposes in tests, e.g. to emit log messages from a forth-coming HTTP server.
This way, our log messages (and those from our stack) will show up in case of errors or test failures, which makes debugging easier.
dlax
force-pushed
the
fixtures
branch
2 times, most recently
from
October 3, 2023 12:59
597f07e
to
198af5e
Compare
blogh
requested changes
Oct 5, 2023
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.
Thanks for this PR. It's really nice.
Can you add an entry about the HTTP server and the code coverage in the
CHANGELOG please ?
Instead of requiring the user to run the test suite with and without the --use-old-replica-state flag, we introduce an 'old_replica_state()' parametrized fixture that is used only when needed (i.e. in test_cluster_{has_replica,node_count}.py).
To be run with 'pytest --cov --cov-report=html'.
Instead of repeating the dependencies.
We introduce a patroni_api fixture, defined in tests/conftest.py, which sets up an HTTP server serving files in a temporary directory. The server is itself defined by the PatroniAPI class; it has a 'routes()' context manager method to be used in actual tests to setup expected responses based on specified JSON files. We set up some logging in order to improve debugging. The direct advantage of this is that PatroniResource.rest_api() method is now covered by the test suite. Coverage before this commit: Name Stmts Miss Cover ----------------------------------------------- check_patroni/__init__.py 3 0 100% check_patroni/cli.py 193 18 91% check_patroni/cluster.py 113 0 100% check_patroni/convert.py 23 5 78% check_patroni/node.py 146 1 99% check_patroni/types.py 50 23 54% ----------------------------------------------- TOTAL 528 47 91% and after this commit: Name Stmts Miss Cover ----------------------------------------------- check_patroni/__init__.py 3 0 100% check_patroni/cli.py 193 18 91% check_patroni/cluster.py 113 0 100% check_patroni/convert.py 23 5 78% check_patroni/node.py 146 1 99% check_patroni/types.py 50 9 82% ----------------------------------------------- TOTAL 528 33 94% In actual test functions, we either invoke patroni_api.routes() to configure which JSON file(s) should be served for each endpoint, or we define dedicated fixtures (e.g. cluster_config_has_changed()) to configure this for several test functions or the whole module. The 'old_replica_state' parametrized fixture is used when needed to adjust such fixtures, e.g. in cluster_has_replica_ok(), to modify the JSON content using cluster_api_set_replica_running() (previously in tests/tools.py, now in tests/__init__.py). The dependency on pytest-mock is no longer needed.
From previous commit, the test suite also type-checks.
blogh
approved these changes
Oct 6, 2023
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.
A series of small changes in tests, to prepare for the large one "Use fake HTTP server for the Patroni API in tests" which replaces the previous mock by a real HTTP server faking the Patroni API.
Should fix #51 as the dependency on pytest-mock is dropped.