From b97ccd7ad87df14afd344383a3008f7e5d2e09a1 Mon Sep 17 00:00:00 2001 From: Cody Schaefer Date: Mon, 24 Aug 2026 10:09:03 -0500 Subject: [PATCH] Fix test env isolation: a popped variable comes back from .env Caught the moment a real LDAP_REQUIRED_GROUP landed in .env: ldap_auth_check's "a just-provisioned account appears in the Admin console list" started failing, consistently, with no code change that could explain it. start() removed LDAP_REQUIRED_GROUP from the subprocess environment so the test could run without a group gate. But the server imports server/db.py, which calls load_dotenv(), and python-dotenv skips only keys ALREADY PRESENT in os.environ - so a popped variable is helpfully restored from the developer's .env inside the child process. The test was quietly running against the real required group, the fake "outsider" account was refused by it, and the account under test was never provisioned. Setting the variable to an empty string fixes it: empty still counts as present, so load_dotenv leaves it alone. Fixed in both harnesses - ldap_auth_check and browser_check, the latter shared by 39 files - and corrected the fix suggested in BL-028, which said to pop MICRON_DB_URL and would therefore not have worked either. Worth stating as a rule: in this repo a test cannot assume an environment variable is ABSENT. .env re-supplies it in any subprocess. Force the value you want; never remove it. ldap_auth_check 32/32, browser_check re-run green. Co-Authored-By: Claude Opus 5 (1M context) --- docs/waves/backlog.md | 8 ++++++-- tests/browser_check.py | 9 ++++++--- tests/ldap_auth_check.py | 10 ++++++---- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/docs/waves/backlog.md b/docs/waves/backlog.md index 07c162e..8aedeb6 100644 --- a/docs/waves/backlog.md +++ b/docs/waves/backlog.md @@ -601,8 +601,12 @@ deliberately deferred. anyone who has ever used the asset picker — the API is genuinely configured, returns real Micron tags, and three checks fail. Nothing is wrong with the app; the test's premise is violated by the environment it runs in. -- **Fix:** pop `MICRON_DB_URL` from the env for that server, exactly as `start_server` - now pops `LDAP_REQUIRED_GROUP` for the same reason (`T10.7`). +- **Fix:** SET `MICRON_DB_URL` empty for that server — do not pop it. `server/db.py` + calls `load_dotenv()` at import, and python-dotenv only skips keys already present in + `os.environ`, so a *popped* variable is restored from the developer's `.env` inside the + subprocess and the test runs against the real catalog anyway. An empty string counts as + present and therefore wins. `start_server` does exactly this for `LDAP_REQUIRED_GROUP` + (`T10.7`), after the pop-based version was caught doing the wrong thing. - **Why not now:** it is not this wave's defect and the fix belongs with whoever owns the asset picker's tests. Recorded so the failure is not mistaken for D13 fallout. - **Suggested wave or follow-up:** next housekeeping pass. diff --git a/tests/browser_check.py b/tests/browser_check.py index 63969cf..0b23f9a 100644 --- a/tests/browser_check.py +++ b/tests/browser_check.py @@ -172,9 +172,12 @@ def start_server(port, db_path): env["DATABASE_URL"] = "sqlite:///" + db_path.replace("\\", "/") env.setdefault("AUTH_SECRET_KEY", "browser-check-secret-not-for-production") env["WP_LDAP_FAKE_DIRECTORY"] = FAKE_DIRECTORY - # No required group: the fake grants "WP-Suite-Users" to everyone, and a test - # asserting the group gate belongs in ldap_auth_check where it can be explicit. - env.pop("LDAP_REQUIRED_GROUP", None) + # SET empty, never pop: server/db.py calls load_dotenv() at import and + # python-dotenv only skips keys already present in os.environ, so a popped + # variable comes back from the developer's .env inside the subprocess. An empty + # string is "present" and therefore wins. The fake grants "WP-Suite-Users" to + # everyone; a test asserting the group gate belongs in ldap_auth_check. + env["LDAP_REQUIRED_GROUP"] = "" proc = subprocess.Popen( [sys.executable, "-m", "uvicorn", "server.app:app", "--host", "127.0.0.1", "--port", str(port), "--log-level", "warning"], diff --git a/tests/ldap_auth_check.py b/tests/ldap_auth_check.py index 8034e49..679f85d 100644 --- a/tests/ldap_auth_check.py +++ b/tests/ldap_auth_check.py @@ -89,10 +89,12 @@ def start(port, db_path, fake, required_group=""): env["DATABASE_URL"] = "sqlite:///" + db_path.replace("\\", "/") env["AUTH_SECRET_KEY"] = SECRET env["WP_LDAP_FAKE_DIRECTORY"] = json.dumps(fake) - if required_group: - env["LDAP_REQUIRED_GROUP"] = required_group - else: - env.pop("LDAP_REQUIRED_GROUP", None) + # SET it empty, never pop it. server/db.py calls load_dotenv() at import, and + # python-dotenv only skips a key that is already present in os.environ — so a + # popped variable is helpfully restored from the developer's .env inside the + # subprocess, and the test silently runs against the real required group. + # An empty string counts as present, so it wins. + env["LDAP_REQUIRED_GROUP"] = required_group or "" proc = subprocess.Popen( [sys.executable, "-m", "uvicorn", "server.app:app", "--host", "127.0.0.1", "--port", str(port), "--log-level", "warning"],