fix(tests): override "test" sentinel in security conftest (#804) - #805
Merged
Conversation
…leanup_unreachable_orphan.py #791 (which added the tests/lint_sys_modules.py lint and its baseline) landed AFTER #783 (which added tests/unit/test_cleanup_unreachable_orphan.py). The baseline was generated without including the orphan-test file's 3 top-level `sys.modules[...] = stub` assignments at lines 64, 76, 79 (docker, services.docker_service, database stubs installed before the test module's services.cleanup_service import). Result: every PR opened against dev fails the lint job on these 3 lines, even when the PR's own changes are clean. Fix: grandfather the file into the baseline at its current count (3). This matches how the same shape is handled elsewhere — e.g. test_watchdog_unit.py (3 violations, the precedent the orphan-test file's own comment cites). The lint script's baseline path is the intended mechanism for accepting pre-existing module-level stub installs that cannot use monkeypatch. Verified locally: python3 tests/lint_sys_modules.py emits zero violations in the real test tree (only .venv site-packages remain, which CI runners don't have). Refs #762, #783, #791 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`tests/conftest.py` setdefaults REDIS_PASSWORD/REDIS_BACKEND_PASSWORD to the literal "test" at global pytest import so backend modules can be imported without real Redis creds. `tests/security/conftest.py` was then using `os.environ.setdefault(...)` to overlay real `.env` values — a no-op because the sentinel was already set. Result: `redis-cli` ran with `-a test` against a healthy stack and the ACL acceptance tests failed for the wrong reason. Pop the sentinel before the `.env` overlay and use direct assignment. Add a regression test that asserts the live env value is not the sentinel and (when `.env` defines the keys) matches the `.env` value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AndriiPasternak31
force-pushed
the
AndriiPasternak31/issue-804
branch
from
May 12, 2026 01:52
018727c to
2296450
Compare
4 tasks
AndriiPasternak31
changed the base branch from
dev
to
AndriiPasternak31/lint-baseline-orphan-test
May 12, 2026 01:54
vybe
requested changes
May 12, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
This PR requires the following change before merge:
- Retarget base branch to
dev—AndriiPasternak31/lint-baseline-orphan-testwas already squash-merged todevas PR #796. Change via the GitHub UI: Edit → Base:dev.
Code is clean and the fix is correct. This is the only blocker.
AndriiPasternak31
changed the base branch from
AndriiPasternak31/lint-baseline-orphan-test
to
dev
May 12, 2026 22:10
Contributor
Author
|
Hi Eugene — base is already |
1 task
vybe
approved these changes
May 14, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
Original blocker resolved. Fix is correct — sentinel pop + direct assignment is the right call. Regression test properly locks it in. LGTM.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
tests/security/conftest.pywas usingos.environ.setdefault(...)to overlay.env, which is a no-op oncetests/conftest.pyhas set the"test"sentinel at global pytest import. The Redis ACL acceptance tests then ranredis-cliwith-a testagainst a healthy stack and failed withNOAUTH— wrong-creds, not a real ACL regression.REDIS_PASSWORD/REDIS_BACKEND_PASSWORDbefore the.envoverlay, then assign directly so real values win.tests/security/test_redis_conftest_regression.pyto lock this in: asserts the live env value isn't the sentinel, and (when.envdefines the keys) matches.envexactly. Skips cleanly when neither.envnor real env vars provide creds.Closes #804.
Test plan
pytest tests/security/test_redis_conftest_regression.pypasses against a stack with real Redis creds in.envtests/security/test_redis_network_isolation.py(the 3 ACL tests) pass against a freshly-started stack.env, no env vars), the security suite still skips cleanly via the existing skip guard🤖 Generated with Claude Code