Skip to content

fix(tests): repair tests and fixtures drifted behind merged changes (#3442, #3272) - #3468

Merged
vybe merged 1 commit into
devfrom
feature/3442-test-suite-drift
Oct 9, 2026
Merged

vybe merged 1 commit into
devfrom
feature/3442-test-suite-drift

Conversation

@trinity-ability

Copy link
Copy Markdown
Contributor

Summary

Test-only change. A full tests/run-full.sh against a freshly rebuilt local stack failed 7 of 10 tiers. Triage found one product bug (filed as #3441) and a set of tests and fixtures that had drifted behind merged product changes or depended on the state of the machine they ran on. This PR fixes the test side.

Fixes #3442
Fixes #3272
Related to #2420 (first two acceptance criteria only — the TRINITY_TEST_ALLOW_DESTRUCTIVE guard, the README note and the guard's own test are not in this PR)

What changed

File Was Now
tests/scheduler_tests/conftest.py Hand-mirrored schedule_executions DDL lacked chain_depth and claim_token; 70 of 243 failed Both columns added; 243 pass
tests/unit/test_deploy_writable_templates.py Evicted six services.* modules and re-imported; teardown restored sys.modules but left the parent-package attributes pointing at the fresh objects, so later files patched one module and the code read another (#3272) Registers each package attribute with monkeypatch before eviction so teardown restores it
tests/test_subscriptions.py test_create_agent_no_subscriptions deleted every subscription on the target instance (#2420) Deletes only the suite's own test- subscriptions; skips with a named reason when operator-owned ones remain. test_create_agent_with_subscription accepts an operator-owned subscription as a valid auto-assign target
tests/test_subscription_auto_switch.py Left auto-switch OFF after the run Module fixture restores the value it found
tests/integration/test_circuit_breaker.py Asserted db.create_operator_queue_item for the dormant alert Asserts the platform-alert seam the alert moved to in #3246 (create_platform_operator_queue_item, one row per subject)
tests/unit/test_1661_sanitizer_linear.py 15 TestChainedPairs cases failed whenever the environment held a sensitive-named variable whose value was the word password (which tests/setup-env.sh produces on an install with that admin password) Autouse fixture pins the agent sanitizer's known-value cache to empty, so the KEY=value pass is what gets tested
tests/test_ip_rate_limit_fix.py 14 setup errors: the minimal fastapi stub had no status, which services.rate_limiter now imports Stub carries the real starlette.status constants
tests/test_operator_queue.py Answered the first pending item with a hardcoded approve; 422 when that item is an approval with its own options Answers with a decision the item offers
tests/test_parallel_task.py Async-mode tests rejected status queued queued accepted as the legitimate first state

Test run report (2026-10-09, dev @ f6bedcd81, all images rebuilt)

Verdict of the run that prompted this: FAILED (run-full.sh exit 1). Preflight was green 5/5.

Tier Result before this PR Cause
unit 86 failed, 24,548 passed Module-eviction contamination (#3272) plus the sanitizer env leak
integration 1 failed, 71 passed Stale dormant-alert assertion
git-sync 23 passed —
security timed out Environment: helper image absent and the host's Docker credential helper hung on the pull. Passes (6 passed, 5 skipped) once the image is local
scheduler 70 failed, 173 passed Fixture drift
agent-server 14 passed —
journeys 16 passed, 3 xfailed —
api 12 failed, 33 errors, 1,700 passed ~23 errors downstream of the suite deleting the instance's subscription mid-run; 14 from the fastapi.status stub; the rest stale assertions and three order-dependent flakes
standalone passed —
postgres 1 failed, 2,330 passed Product bug, #3441

One finding was reclassified during the fix: the dormant-alert failure looked like a possible product regression (NoEventLoopError in the log) but is a stale test — the alert is filed correctly through the new seam; the logged error is only the follow-up broadcast having no event loop in a sync test.

Verification

Run from this branch:

  • pytest tests/scheduler_tests — 243 passed (was 70 failed)
  • pytest tests/unit/ (full tier, setup-env.sh sourced) — 1 failed, 24,878 passed (was 86 failed). The one failure, test_1968_trigger_execution_id.py::test_service_does_not_recreate_a_passed_execution (services.monitoring_alerts has no attribute MonitoringAlertService), passes 3 of 3 in isolation — a separate order-dependent contamination with a different culprit, not addressed here
  • test_deploy_writable_templates.py followed by each former victim (test_ent477_definitions_endpoint, test_ent477_git_refresh_hook, test_ent666_objective_join, test_retention_floor) — all pass
  • TRINITY_TEST_PASSWORD=password pytest tests/unit/test_1661_sanitizer_linear.py — 185 passed
  • pytest tests/integration/test_circuit_breaker.py — 34 passed
  • pytest tests/test_ip_rate_limit_fix.py — 14 passed
  • pytest tests/test_subscriptions.py tests/test_subscription_auto_switch.py against a live stack holding a real subscription — 27 passed, 1 skipped; the subscription and the auto-switch value were unchanged afterwards
  • pytest tests/test_avatars.py "tests/test_operator_queue.py::TestRespondToQueueItem" — 37 passed
  • pytest tests/security — 6 passed, 5 skipped
  • tests/lint_sys_modules.py — no new violations

Not re-run: the full run-full.sh end to end on this branch, and test_parallel_task.py (the change only widens an accepted-status list).

Not fixed here

🤖 Generated with Claude Code

…ifted (#3442, #3272, #2420)

A full suite run against a freshly rebuilt stack failed 7 of 10 tiers; one
product bug came out of it (#3441) and the rest was test rot:

- scheduler fixture: add `chain_depth` and `claim_token` to the hand-mirrored
  `schedule_executions` DDL (70 failures -> 243 passing)
- test_deploy_writable_templates: restore the parent-package attributes the
  re-import rebinds, not only the sys.modules entries, so later files stop
  patching one module object while the code reads another (#3272)
- test_subscriptions: delete only the suite's own `test-` subscriptions and
  skip the no-subscription case when the operator has real ones, instead of
  wiping every subscription on the instance; test_subscription_auto_switch
  restores the setting it found (#2420, first two criteria)
- test_circuit_breaker: assert the platform-alert seam the dormant alert
  moved to in #3246
- test_1661_sanitizer_linear: pin the agent sanitizer's known-value cache so
  an ambient TRINITY_TEST_PASSWORD cannot redact the key under test
- test_ip_rate_limit_fix: give the fastapi stub the `status` module
  services.rate_limiter now imports
- test_operator_queue: answer with a decision the item actually offers
- test_parallel_task: `queued` is a legitimate first status

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train/20261009-2324-b (#3504)

@vybe
vybe merged commit 36711e7 into dev Oct 9, 2026
21 checks passed
vybe pushed a commit that referenced this pull request Oct 21, 2026
…a fresh module copy (#3390) (#3848)

#3468 (36711e7) already fixed the 40 failures #3390 named: putting the
pre-#3468 test_deploy_writable_templates.py back still fails 41 tests
(ent666 x21, ent477 git_refresh_hook x10, ent477 definitions_endpoint x6,
retention_floor x4). This closes the class those 40 belonged to.

A test that evicts `pkg.mod` and re-imports it rebinds the package
attribute `pkg.mod` to the fresh copy, and monkeypatch.delitem, patch.dict
and every hand-written snapshot restore put back only the sys.modules
entry. The rest of the run then holds two copies of one module: a later
test patches one while the code under test reads the other. Five
recurrences so far (#3272, #3470, #2572, trinity-enterprise#752, #2859).
A probe over the whole unit island found 64 files that leave such a split.

- tests/module_split_guard.py: scan() finds a split, Guard repairs it,
  unbind() drops a stale binding, evict() is the eviction that leaves no
  split behind (a package takes its loaded submodules with it, #2859).
- tests/unit/conftest.py runs the Guard around every test, after the
  item's last teardown:
  - a split whose sys.modules entry is still the pre-test module is
    repaired, by putting the attribute back to its pre-test value;
  - a split the test made by replacing the entry fails the file's last
    test, judged once its module fixtures have torn down, so a module
    fixture that restores its own copy is not a leak;
  - collection-time splits are left alone. Healing them toward
    sys.modules broke four test_ent692_skill_sources_mcp tests, because
    the entry was a test file's private copy.
- TRINITY_SPLIT_GUARD=off switches the guard off. It is the control arm
  of the guard's end-to-end test.
- The conftest's own _POP_PREFIXES pops now drop the package binding of
  backend modules. Before, `from services import task_execution_service`
  (pull_coordination_service.py) kept handing out the popped copy.
- tests/lint_sys_modules.py stops recommending raw monkeypatch.delitem.
  A delitem of a dotted or computed name is now flagged outside
  tests/unit/, where nothing repairs it (3 root files baselined), and the
  hint points at evict().
- Fixed the two files that left a copy in sys.modules for good:
  test_session_operations (db.sessions) and test_audit_retention_prune
  (db.audit). Both now use monkeypatch.setitem.
- test_deploy_writable_templates.py: #3468's inline loop becomes evict().

The other 62 polluting files need no edit: the guard repairs them.

Proof on the real class, with the pre-#3468 polluter back plus the 4
victim files, -p no:randomly: 121 passed with the guard; 41 failed with
TRINITY_SPLIT_GUARD=off.

Fixes #3390

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants