Turn --use-old-replica-state into a parametrized fixture

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).
2023-10-03 14:29:21 +02:00 · 2023-10-03 14:29:21 +02:00 · 34f576ea0f
parent fea89041b8
commit 34f576ea0f
5 changed files with 59 additions and 63 deletions
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@ -61,21 +61,15 @@ erroneously.
 The tests are executed automatically for each PR using the ci (see
 `.github/workflow/lint.yml` and `.github/workflow/tests.yml`).

-Running the tests manually:
+Running the tests,

-* Using patroni's nominal replica state of `streaming` (since v3.0.4):
+* manually:

  ```bash
-  pytest ./tests
+  pytest tests
  ```

-* Using patroni's nominal replica state of `running` (before v3.0.4):
-
-  ```bash
-  pytest --use-old-replica-state ./tests
-  ```
-
-* Using tox:
+* or using tox:

  ```bash
  tox -e lint    # mypy + flake8 + black + isort ° codespell
@ -83,9 +77,9 @@ Running the tests manually:
  tox -e py      # pytests and "lint" tests for the default version of python
  ```

-Please note that when dealing with any service that checks the state of a node
-in patroni's `cluster` endpoint, the corresponding JSON test file must be added
-in `./tests/tools.py`.
+Please note that when dealing with any service that checks the state of a node,
+the related tests must use the `old_replica_state` fixture to test with both
+old (pre 3.0.4) and new replica states.

 A bash script, `check_patroni.sh`, is provided to facilitate testing all
 services on a Patroni endpoint (`./vagrant/check_patroni.sh`). It requires one
--- a/tests/conftest.py
+++ b/tests/conftest.py
@ -8,25 +8,17 @@ from pytest_mock import MockerFixture
 from .tools import my_mock


-def pytest_addoption(parser: Any) -> None:
-    """
-    Add CLI options to `pytest` to pass those options to the test cases.
-    These options are used in `pytest_generate_tests`.
-    """
-    parser.addoption("--use-old-replica-state", action="store_true", default=False)
-
-
-def pytest_generate_tests(metafunc: Any) -> None:
-    metafunc.parametrize(
-        "use_old_replica_state", [metafunc.config.getoption("use_old_replica_state")]
-    )
+@pytest.fixture(
+    params=[False, True],
+    ids=lambda v: "new-replica-state" if v else "old-replica-state",
+)
+def old_replica_state(request: Any) -> Any:
+    return request.param


@pytest.fixture
-def fake_restapi(
-    mocker: MockerFixture, use_old_replica_state: bool
-) -> Callable[..., Any]:
-    return partial(my_mock, mocker, use_old_replica_state=use_old_replica_state)
+def fake_restapi(mocker: MockerFixture) -> Callable[..., Any]:
+    return partial(my_mock, mocker)


@pytest.fixture
--- a/tests/test_cluster_has_replica.py
+++ b/tests/test_cluster_has_replica.py
@ -4,8 +4,10 @@ from check_patroni.cli import main


 # TODO Lag threshold tests
-def test_cluster_has_relica_ok(runner: CliRunner, fake_restapi) -> None:
-    fake_restapi("cluster_has_replica_ok")
+def test_cluster_has_relica_ok(
+    runner: CliRunner, fake_restapi, old_replica_state: bool
+) -> None:
+    fake_restapi("cluster_has_replica_ok", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main, ["-e", "https://10.20.199.3:8008", "cluster_has_replica"]
    )
@ -17,9 +19,9 @@ def test_cluster_has_relica_ok(runner: CliRunner, fake_restapi) -> None:


 def test_cluster_has_replica_ok_with_count_thresholds(
-    runner: CliRunner, fake_restapi
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_has_replica_ok")
+    fake_restapi("cluster_has_replica_ok", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -40,9 +42,9 @@ def test_cluster_has_replica_ok_with_count_thresholds(


 def test_cluster_has_replica_ok_with_sync_count_thresholds(
-    runner: CliRunner, fake_restapi
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_has_replica_ok")
+    fake_restapi("cluster_has_replica_ok", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -61,9 +63,9 @@ def test_cluster_has_replica_ok_with_sync_count_thresholds(


 def test_cluster_has_replica_ok_with_count_thresholds_lag(
-    runner: CliRunner, fake_restapi
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_has_replica_ok_lag")
+    fake_restapi("cluster_has_replica_ok_lag", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -86,9 +88,9 @@ def test_cluster_has_replica_ok_with_count_thresholds_lag(


 def test_cluster_has_replica_ko_with_count_thresholds(
-    runner: CliRunner, fake_restapi
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_has_replica_ko")
+    fake_restapi("cluster_has_replica_ko", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -109,9 +111,9 @@ def test_cluster_has_replica_ko_with_count_thresholds(


 def test_cluster_has_replica_ko_with_sync_count_thresholds(
-    runner: CliRunner, fake_restapi
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_has_replica_ko")
+    fake_restapi("cluster_has_replica_ko", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -132,9 +134,9 @@ def test_cluster_has_replica_ko_with_sync_count_thresholds(


 def test_cluster_has_replica_ko_with_count_thresholds_and_lag(
-    runner: CliRunner, fake_restapi
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_has_replica_ko_lag")
+    fake_restapi("cluster_has_replica_ko_lag", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
--- a/tests/test_cluster_node_count.py
+++ b/tests/test_cluster_node_count.py
@ -4,14 +4,14 @@ from check_patroni.cli import main


 def test_cluster_node_count_ok(
-    runner: CliRunner, fake_restapi, use_old_replica_state: bool
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_node_count_ok")
+    fake_restapi("cluster_node_count_ok", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main, ["-e", "https://10.20.199.3:8008", "cluster_node_count"]
    )
    assert result.exit_code == 0
-    if use_old_replica_state:
+    if old_replica_state:
        assert (
            result.output
            == "CLUSTERNODECOUNT OK - members is 3 | healthy_members=3 members=3 role_leader=1 role_replica=2 state_running=3\n"
@ -24,9 +24,9 @@ def test_cluster_node_count_ok(


 def test_cluster_node_count_ok_with_thresholds(
-    runner: CliRunner, fake_restapi, use_old_replica_state: bool
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_node_count_ok")
+    fake_restapi("cluster_node_count_ok", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -44,7 +44,7 @@ def test_cluster_node_count_ok_with_thresholds(
        ],
    )
    assert result.exit_code == 0
-    if use_old_replica_state:
+    if old_replica_state:
        assert (
            result.output
            == "CLUSTERNODECOUNT OK - members is 3 | healthy_members=3;@2;@1 members=3;@1;@2 role_leader=1 role_replica=2 state_running=3\n"
@ -57,9 +57,11 @@ def test_cluster_node_count_ok_with_thresholds(


 def test_cluster_node_count_healthy_warning(
-    runner: CliRunner, fake_restapi, use_old_replica_state: bool
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_node_count_healthy_warning")
+    fake_restapi(
+        "cluster_node_count_healthy_warning", use_old_replica_state=old_replica_state
+    )
    result = runner.invoke(
        main,
        [
@ -73,7 +75,7 @@ def test_cluster_node_count_healthy_warning(
        ],
    )
    assert result.exit_code == 1
-    if use_old_replica_state:
+    if old_replica_state:
        assert (
            result.output
            == "CLUSTERNODECOUNT WARNING - healthy_members is 2 (outside range @0:2) | healthy_members=2;@2;@1 members=2 role_leader=1 role_replica=1 state_running=2\n"
@ -85,8 +87,12 @@ def test_cluster_node_count_healthy_warning(
        )


-def test_cluster_node_count_healthy_critical(runner: CliRunner, fake_restapi) -> None:
-    fake_restapi("cluster_node_count_healthy_critical")
+def test_cluster_node_count_healthy_critical(
+    runner: CliRunner, fake_restapi, old_replica_state: bool
+) -> None:
+    fake_restapi(
+        "cluster_node_count_healthy_critical", use_old_replica_state=old_replica_state
+    )
    result = runner.invoke(
        main,
        [
@ -107,9 +113,9 @@ def test_cluster_node_count_healthy_critical(runner: CliRunner, fake_restapi) ->


 def test_cluster_node_count_warning(
-    runner: CliRunner, fake_restapi, use_old_replica_state: bool
+    runner: CliRunner, fake_restapi, old_replica_state: bool
 ) -> None:
-    fake_restapi("cluster_node_count_warning")
+    fake_restapi("cluster_node_count_warning", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
@ -123,7 +129,7 @@ def test_cluster_node_count_warning(
        ],
    )
    assert result.exit_code == 1
-    if use_old_replica_state:
+    if old_replica_state:
        assert (
            result.stdout
            == "CLUSTERNODECOUNT WARNING - members is 2 (outside range @0:2) | healthy_members=2 members=2;@2;@1 role_leader=1 role_replica=1 state_running=2\n"
@ -135,8 +141,10 @@ def test_cluster_node_count_warning(
        )


-def test_cluster_node_count_critical(runner: CliRunner, fake_restapi) -> None:
-    fake_restapi("cluster_node_count_critical")
+def test_cluster_node_count_critical(
+    runner: CliRunner, fake_restapi, old_replica_state: bool
+) -> None:
+    fake_restapi("cluster_node_count_critical", use_old_replica_state=old_replica_state)
    result = runner.invoke(
        main,
        [
--- a/tests/tools.py
+++ b/tests/tools.py
@ -29,10 +29,10 @@ def my_mock(
        if status != 200:
            raise APIError("Test en erreur pour status code 200")
        if json_file:
-            if use_old_replica_state and (
-                json_file.startswith("cluster_has_replica")
-                or json_file.startswith("cluster_node_count")
-            ):
+            if use_old_replica_state:
+                assert json_file.startswith(
+                    "cluster_has_replica"
+                ) or json_file.startswith("cluster_node_count")
                return cluster_api_set_replica_running(getjson(json_file))
            return getjson(json_file)
        return None