test: refactor webui test flow

Having two different fixtures `binary_path` and
`path_to_binary_with_webui` results in weird failures in CI:

```
FAILED test/int/test_couple.py::test_exit_after_apply - conftest.TarantoolError:
    ('ER_NO_SUCH_PROC', "Procedure 'pico._inject_error' is not defined")
```

The root cause is that one `cargo build` overrides another with
different features enabled. To resolve the issue this patch
significantly refactors the test flow:

1. Web UI tests are skipped by default. One can enable it explicitly
   with `pytest --with-webui`

2. New fixture `cargo_build` behavior depends on environment:
   - In local dev environment it rebuilds the project with required features
   - In CI it skips rebuilding and runs tests against the existing build

3. The `path_to_binary_with_webui` fixture is removed.

See also:

- https://stackoverflow.com/a/55769818/1182787
- https://docs.pytest.org/en/7.4.x/how-to/writing_hook_functions.html
- https://docs.pytest.org/en/7.4.x/how-to/mark.html
- https://docs.pytest.org/en/7.4.x/example/simple.html#control-skipping-of-tests-according-to-command-line-option

.gitlab-ci.yml

@@ -184,13 +184,17 @@ test-linux:
 
     - make lint
 
+    # There are no Rust tests for `webui` feature, it's checked in pytest
     - cargo build --locked
-    - cargo build --features webui --locked
-    # There are no Rust tests for `webui` feature.
-    # It will be checked during integration tests.
     - cargo test --locked
 
-    - pipenv run pytest --numprocesses auto -v --junitxml=junit_pytest.xml
+    - cargo build --features webui,error_injection --locked
+    - >
+      pipenv run pytest -v
+      --numprocesses auto
+      --junitxml=junit_pytest.xml
+      --with-webui
+
     - |
       # Save cache
       if [ "$CI_COMMIT_BRANCH" == "$CI_DEFAULT_BRANCH" ]; then

pytest.ini

@@ -3,3 +3,5 @@ testpaths =
     test/int
 addopts =
     -v --showlocals
+markers =
+    webui

test/conftest.py

@@ -67,6 +67,23 @@ def pytest_addoption(parser: pytest.Parser):
         default=False,
         help="Whether gather flamegraphs or not (for benchmarks only)",
     )
+    parser.addoption(
+        "--with-webui",
+        action="store_true",
+        default=False,
+        help="Whether to run Web UI tests",
+    )
+
+
+def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item]):
+    # https://docs.pytest.org/en/7.4.x/how-to/writing_hook_functions.html
+    # https://docs.pytest.org/en/7.4.x/example/simple.html#control-skipping-of-tests-according-to-command-line-option
+
+    if not config.getoption("--with-webui"):
+        skip = pytest.mark.skip(reason="run: pytest --with-webui")
+        for item in items:
+            if "webui" in item.keywords:
+                item.add_marker(skip)
 
 
 @pytest.fixture(scope="session")
@@ -1444,25 +1461,32 @@ class PortalStorage:
 
 
 @pytest.fixture(scope="session")
-def binary_path() -> str:
+def binary_path(cargo_build: None) -> str:
     """Path to the picodata binary, e.g. "./target/debug/picodata"."""
-    assert (
-        subprocess.call(["cargo", "build", "--features", "error_injection"]) == 0
-    ), "cargo build failed"
     metadata = subprocess.check_output(["cargo", "metadata", "--format-version=1"])
     target = json.loads(metadata)["target_directory"]
     return os.path.realpath(os.path.join(target, "debug/picodata"))
 
 
 @pytest.fixture(scope="session")
-def path_to_binary_with_webui() -> str:
-    """Path to the picodata binary built with webui feature, e.g. "./target/debug/picodata"."""
-    assert (
-        subprocess.call(["cargo", "build", "--features", "webui"]) == 0
-    ), "cargo build failed"
-    metadata = subprocess.check_output(["cargo", "metadata", "--format-version=1"])
-    target = json.loads(metadata)["target_directory"]
-    return os.path.realpath(os.path.join(target, "debug/picodata"))
+def cargo_build(pytestconfig: pytest.Config) -> None:
+    """Run cargo build before tests. Skipped in CI"""
+
+    # Start test logs with a newline. This makes them prettier with
+    # `pytest -s` (a shortcut for `pytest --capture=no`)
+    eprint("")
+
+    if os.environ.get("CI") is not None:
+        eprint("Skipping cargo build")
+        return
+
+    features = ["error_injection"]
+    if bool(pytestconfig.getoption("with_webui")):
+        features.append("webui")
+
+    cmd = ["cargo", "build", "--features", ",".join(features)]
+    eprint(f"Running {cmd}")
+    assert subprocess.call(cmd) == 0, "cargo build failed"
 
 
 @pytest.fixture(scope="session")
@@ -1489,24 +1513,6 @@ def cluster(
     cluster.kill()
 
 
-@pytest.fixture
-def cluster_with_webui(
-    path_to_binary_with_webui, tmpdir, cluster_ids, port_range
-) -> Generator[Cluster, None, None]:
-    """Return a `Cluster` object capable of deploying test clusters with webui feature enabled."""
-    base_port, max_port = port_range
-    cluster = Cluster(
-        binary_path=path_to_binary_with_webui,
-        id=next(cluster_ids),
-        data_dir=tmpdir,
-        base_host=BASE_HOST,
-        base_port=base_port,
-        max_port=max_port,
-    )
-    yield cluster
-    cluster.kill()
-
-
 @pytest.fixture
 def instance(cluster: Cluster) -> Generator[Instance, None, None]:
     """Returns a deployed instance forming a single-node cluster."""

test/int/test_http_server.py

@@ -29,13 +29,9 @@ def test_http_routes(instance: Instance):
         assert response.read() == b"world"
 
 
-def test_webui(cluster_with_webui: Cluster):
-    c = cluster_with_webui
-    instance = c.add_instance(wait_online=False)
-    http_listen = f"{c.base_host}:{c.base_port+80}"
-    instance.env["PICODATA_HTTP_LISTEN"] = http_listen
-    instance.start()
-    instance.wait_online()
+@pytest.mark.webui
+def test_webui(instance: Instance):
+    http_listen = instance.env["PICODATA_HTTP_LISTEN"]
     with urlopen(f"http://{http_listen}/") as response:
         assert response.headers.get("content-type") == "text/html"