mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-09-07 02:25:21 +03:00
fix: make the patched get_default_project_directory order-safe in tests
A product module first-imported while run_around_tests' autouse monkeypatch of gns3server.utils.path.get_default_project_directory is active (e.g. 'from gns3server.api.server import app' inside a test body) binds the patched lambda into its own namespace forever: module-level from-imports capture the object by value. Once the patch is reverted, that module keeps calling the stale lambda, which closed over the very test's tmppath — a directory deleted at that test's teardown. Every later test hitting psutil.disk_usage(get_default_project_directory()) then fails with FileNotFoundError, but only when the importing test runs before the API test files (full-suite collection order hides it). The replacement now resolves Config.instance().settings at call time instead of closing over the tmppath, so a frozen reference stays correct; the test that triggered this imports gns3_app at module top so no product import ever happens inside a patched window. Also pins pytest-random-order (inert without --random-order) and documents the order-independence rules for new tests in the gns3-api-test-writing skill, including the known-red legacy files whose tests share rows sequentially.
This commit is contained in:
parent
0119373690
commit
51b3c81859
@ -56,3 +56,18 @@ Always pass `secret_key=DEFAULT_JWT_SECRET_KEY`: the autouse `run_around_tests`
|
||||
## Failure-Diagnosis Heuristic
|
||||
|
||||
**Passes in isolation, fails in a class run → suspect shared fixture state first** (`base_client.headers`, the `Config` singleton, class-scoped DB rows) — never the product code. Reproduce with `-k "test_a or test_b"` pairs to find the polluting test. Do not add debug prints to product code to chase test-order issues; make the test order-independent with explicit per-request headers instead.
|
||||
|
||||
## Order Independence
|
||||
|
||||
The suite runs in collection order by default, and a full run is green — but that hides order dependencies. Two real incident classes so far:
|
||||
|
||||
1. **The frozen from-import** (fixed 2026-09): `run_around_tests` monkeypatches `gns3server.utils.path.get_default_project_directory` with a lambda. A product module first-imported *while that patch is active* (e.g. `from gns3server.api.server import app` written inside a test body) freezes the patched lambda into its namespace forever — later tests then get a deleted tmpdir path (`FileNotFoundError` from `psutil.disk_usage`). The patched lambda now resolves `Config.instance()` at call time, so freezing is benign — keep it that way.
|
||||
2. **Sequential-scenario DB tests**: `tests/api/routes/controller/test_users.py`, `test_roles.py`, `test_pools.py`, `test_templates.py`, `test_images.py`, `test_groups.py`, `test_appliances.py`, `test_acl.py` and `tests/controller/test_rbac.py` build shared rows across tests within a file (a test asserts on users/roles created by earlier tests). They are known-red under any reordering — do not copy this pattern into new files.
|
||||
|
||||
### Rules for new tests
|
||||
|
||||
- **Import product modules at test-module top level**, never first-import inside a test body (an autouse fixture's monkeypatches are live there, and module import executes product `from`-imports).
|
||||
- **Autouse patch replacements must resolve state at call time** (`Config.instance().settings...`), never close over test-local values (tmppaths, fixture objects) — a closed-over value survives into other tests if the replacement object gets frozen anywhere.
|
||||
- **Verify a new test file is order-independent**: `venv/bin/python -m pytest tests/<new_file>.py --random-order --random-order-seed=1 -q` (and a second seed). It must pass shuffled. `pytest-random-order` is pinned in `dev-requirements.txt`; it is inert unless `--random-order` is passed.
|
||||
- A test that needs specific rows creates them itself (or via a fixture) — never relies on rows another test in the file created.
|
||||
- Diagnosing a suspected order bug: rerun the exact failing pair with the seed printed by `--random-order` (`--random-order-seed=<n>` reproduces it), then bisect to the polluting test.
|
||||
|
||||
@ -3,4 +3,5 @@ flake8==7.3.0
|
||||
pytest-timeout==2.4.0
|
||||
pytest-asyncio==1.4.0
|
||||
httpx==0.28.1
|
||||
httpx_ws==0.7.2 # upgrading leads to failures in tests
|
||||
httpx_ws==0.7.2 # upgrading leads to failures in tests
|
||||
pytest-random-order==1.2.0 # opt-in: --random-order --random-order-bucket=global (legacy suites are not shuffle-clean yet, see .claude/skills/gns3-api-test-writing)
|
||||
@ -418,7 +418,13 @@ def run_around_tests(monkeypatch, config, port_manager):
|
||||
# avoid monitoring for new images while testing
|
||||
config.settings.Server.auto_discover_images = False
|
||||
|
||||
monkeypatch.setattr("gns3server.utils.path.get_default_project_directory", lambda *args: os.path.join(tmppath, 'projects'))
|
||||
# Resolve the projects directory from the Config singleton at call time instead of
|
||||
# closing over this test's tmppath: a from-import executed anywhere while this
|
||||
# patch is active (e.g. the first import of gns3server.api.server from inside a
|
||||
# test body) freezes the patched object into the importing module's namespace
|
||||
# forever, and a closed-over path would then point at a deleted directory in
|
||||
# every later test (order-dependent FileNotFoundError in psutil.disk_usage).
|
||||
monkeypatch.setattr("gns3server.utils.path.get_default_project_directory", lambda *args: Config.instance().settings.Server.projects_path)
|
||||
|
||||
# Force sys.platform to the original value. Because it seems not be restored correctly after each test
|
||||
sys.platform = sys.original_platform
|
||||
|
||||
@ -25,6 +25,7 @@ from unittest.mock import patch, MagicMock
|
||||
|
||||
from gns3server.controller.project import Project
|
||||
from gns3server.controller.compute import Compute
|
||||
from gns3server.api.server import app as gns3_app
|
||||
from gns3server.controller.controller_error import (
|
||||
ControllerError,
|
||||
ControllerNotFoundError,
|
||||
@ -581,8 +582,10 @@ async def test_connect_notification_poison_frame_autoreconnects(compute, monkeyp
|
||||
compute._http_session = session
|
||||
|
||||
# allow the reconnection to be scheduled during the test
|
||||
# (import gns3_app at module top: a first import from inside a test body
|
||||
# would execute module-level from-imports while the autouse fixture's
|
||||
# monkeypatches are active, freezing patched objects into namespaces)
|
||||
monkeypatch.delattr(sys, "_called_from_test", raising=False)
|
||||
from gns3server.api.server import app as gns3_app
|
||||
monkeypatch.setattr(gns3_app.state, "exiting", False)
|
||||
|
||||
async def fake_connect():
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user