Skip to content

test: register a gate module before executing it - #389

Merged
mergify[bot] merged 1 commit into
mainfrom
fix/gate-loader-registers-module
Sep 11, 2026
Merged

test: register a gate module before executing it#389
mergify[bot] merged 1 commit into
mainfrom
fix/gate-loader-registers-module

Conversation

@EdbertChan

@EdbertChan EdbertChan commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Summary

_load in the gate-docstring test builds a module with module_from_spec but never registers it in sys.modules. On Python 3.12+, @dataclass resolves its own module through sys.modules[cls.__module__].__dict__ when annotations are strings, so that lookup returns None and the decorator raises AttributeError before any assertion runs. CI pins Python 3.9, where the lookup takes a different path, so CI cannot see this.

Before and After

The same probe gate (from __future__ import annotations plus one @dataclass) loaded through this test file's _load on Python 3.12.3, with python3 -c 'from pathlib import Path; from tests.test_check_docstrings_match_regexes import _load; m = _load(Path("/tmp/pr389/check_dataclass_probe.py")); print("loaded:", m.Promise("x"))', run once on base 28f750a and once on head 9b18d47:

Before: AttributeError: 'NoneType' object has no attribute '__dict__'. Did you mean: '__dir__'?   (exit 1)
After:  loaded: Promise(text='x')   (exit 0)

Review Claim

A gate module that uses @dataclass under postponed annotations loads successfully in this test's loader.

Review Lane

behavior

Review Unit

engine-runtime

Safety Invariant

Registering the module under its spec name is the documented importlib recipe and does not change which gates are collected or what they assert. No gate currently on main uses @dataclass, so no existing result changes — the fix is for the next one that does.

Slice Rationale

One line in _load plus the test that proves it. No gate is edited.

Non-goals

  • Raising CI's Python version. That is a separate decision with its own blast radius; this fix makes the test correct on both.
  • Adding promise declarations to any gate.

Test Plan

New TestGateLoader.test_a_gate_using_dataclass_loads writes a probe gate that uses from __future__ import annotations plus @dataclass — the shape that triggers the failure — and loads it.

Without sys.modules[spec.name] = module:

Ran 1 test in 0.002s
FAILED (errors=1)

With it:

Ran 1 test in 0.002s
OK

The probe needs the postponed-annotations import: a first version without it passed with and without the fix, proving nothing, and was corrected.

Whole file, on this branch (Python 3.14.7 locally):

$ python3 -m unittest tests.test_check_docstrings_match_regexes -q
Ran 5 tests in 0.229s
OK
$ python3 engine/skills/make-pr/scripts/preflight.py --base origin/main
ok      preflight passed

Found while rebasing #371, whose new scripts/check_detector_backtested.py uses @dataclass three times and crashed this test's setUpClass locally while CI reported green.

Revert Plan

git revert this commit. The loader stops registering the module and any gate using @dataclass crashes the test again on Python 3.12+; none does on main today.

🤖 Generated with Claude Code

https://claude.ai/code/session_013sYZswUQ3HEnUEdVWrUbvq


Note

Low Risk
Test-only loader change with no production or CI Python pin changes; behavior for current gates on main is unchanged.

Overview
Fixes dynamic gate loading in tests/test_check_docstrings_match_regexes.py so it matches the documented importlib pattern and works on Python 3.12+.

_load now assigns sys.modules[spec.name] = module before exec_module. Without that registration, a gate using from __future__ import annotations and @dataclass can fail during import because the decorator looks up the module in sys.modules and gets None—a failure CI on Python 3.9 would not hit.

Adds TestGateLoader.test_a_gate_using_dataclass_loads, which writes a minimal probe gate with postponed annotations and @dataclass, loads it via _load, and asserts flags_exemplar behavior. Existing promise checks are unchanged; no production gates are modified.

Reviewed by Cursor Bugbot for commit 9b18d47. Bugbot is set up for automated code reviews on this repo. Configure here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013sYZswUQ3HEnUEdVWrUbvq
Change-Id: I8b93e3c486069b3c04e42b40193f35a17a15a3d6
@cursor

cursor Bot commented Sep 11, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_09d014d1-3d39-45ec-9776-ab1cca613b04)

@mergify

mergify Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

@EdbertChan

Copy link
Copy Markdown
Owner Author

@Mergifyio queue

@mergify

mergify Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 5 minutes 30 seconds in the queue, including 4 minutes 23 seconds running CI.

Required conditions to merge
  • check-success = lint
  • check-success = test

@mergify mergify Bot added the queued label Sep 11, 2026
@mergify
mergify Bot merged commit 8f33472 into main Sep 11, 2026
4 checks passed
@mergify mergify Bot removed the queued label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant