test: simplify registry import reload coverage

[daryllimyt]

Summary

• remove the permanently skipped reproduction of the historical import race
• remove the stress test that imported and evicted real tracecat_registry modules
• replace the remaining race harness with focused tests alongside import_and_reload
• verify both reload-failure preservation and process-wide reload serialization

Why

The deleted test_import_reload_no_race_with_lock did not actually prove that the process-wide lock was effective. It still passed when _import_reload_lock was replaced with contextlib.nullcontext(), so removing the lock would not have caused the regression test to fail.

The file also contained:

• a permanently skipped test of deliberately unsafe historical behavior
• a pop-window test that waited two seconds for an event that should never occur
• repeated mutation of real tracecat_registry entries in sys.modules, adding broad state and timing dependence to a unit test

Registry sync is now subprocess-isolated, but import_and_reload is still used by local-repository and recovery paths. This PR therefore keeps the relevant contracts and tests them directly:

1. A reload failure must return and preserve the existing module without attempting a fresh import.
2. Two concurrent reload calls must not enter importlib.reload simultaneously.

The new serialization test fails when the lock is disabled, unlike the old stress test. In the local timing run, the removed tests accounted for about 4.8 seconds of test-call time; the new concurrency test completes in about 0.1 seconds.

Validation

• uv run pytest tests/unit/test_registry_repository.py -q --durations=10
• uv run ruff check tests/unit/test_registry_repository.py
• uv run ruff format --check tests/unit/test_registry_repository.py
• uv run basedpyright tests/unit/test_registry_repository.py
• mutation check: confirmed the new serialization test fails when _import_reload_lock is disabled

---

Summary by cubic

Simplifies registry import/reload tests by removing the flaky race harness and adding fast, focused tests that verify reload failure handling and process-wide serialization.

• Refactors
  • Removed tests/unit/test_registry_import_race.py and its flaky race/stress cases against tracecat_registry.
  • Added two unit tests alongside import_and_reload:
    • Preserves the existing module on reload failure (no fresh import_module).
    • Serializes concurrent reloads so importlib.reload never runs in parallel.
  • New serialization test fails when the process-wide lock is disabled, and reduces test time from ~4.8s to ~0.1s.

^{Written for commit 076a714. Summary will update on new commits.}

[daryllimyt]

@codex review

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 076a714.

Security Overview

• 🔎 Scanned files: 2 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/00dffd6c-8834-4dc9-b6d8-b44cd1622986?scanId=93033c70-3aba-469a-974a-45cb57656f47&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type	Relevant files
Refactor	► tests/unit/test_registry_import_race.py     Delete file (removed tests focusing on registry import/reload race)
Enhancement	► tests/unit/test_registry_repository.py     Add new tests for import_and_reload behavior under reload failure and concurrent reloads

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 076a7144fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Wait until the second reload actually contends

The event is signaled before the worker calls import_and_reload, so it does not prove the second thread has attempted to acquire the reload lock. If that worker is descheduled after second_call_started.set()—for example on a loaded CI runner—the 100 ms check can expire, the first reload is released, and the second then runs after it finishes; max_active_reloads remains 1 even with _import_reload_lock removed, allowing the regression this test is intended to catch to pass.

Useful? React with 👍 / 👎.