Unify fixture invocation

[hyperrealist]

This experimental PR addresses concerns in #1057.

• implement docker-outside-docker using @gilesknap's example https://github.com/gilesknap/docker-outside-docker
• re-implement CI system test services as fixtures

What this accomplishes:

• This should now allow user to run pytest tests/system_tests inside devcontainer if needed.
• Exact sane fixture mechanism is used on both the CI and the devcontainer.
• User is provided with an interactive script to start services manually, if need be.

[coretl]

Haven't reviewed the python code yet, but here's a review of the devcontainer and ci...

[vredchenko]

The following review was auto-generated by Claude Code:

---

PR Review Summary

Thanks for this PR! The unified fixture approach is a significant improvement to the testing infrastructure. The docker-outside-docker pattern is well-implemented and the overall architecture is sound. 🎉

✅ Strengths

• Elegant architecture: The docker_composer fixture is a nice generic abstraction
• Improved DX: Unified testing between CI and local devcontainer environments
• CI simplification: Cleaner workflows with the removal of the needs-services matrix
• Good escape hatch: The external_dependencies.sh script for manual service management

🔴 Critical Issues

1. Race Condition in Process Termination (tests/conftest.py:404-417)

After sending SIGTERM, the code immediately calls process.wait() without checking if the process exited. This could hang if the process doesn't exit cleanly.

Recommendation: Add graceful shutdown with escalation to SIGKILL:

try:
    os.killpg(os.getpgid(process.pid), signal.SIGTERM)
    process.wait(timeout=5.0)  # Grace period for clean exit
except TimeoutExpired:
    os.killpg(os.getpgid(process.pid), signal.SIGKILL)
    process.wait(timeout=stop_timeout)
except ProcessLookupError:
    pass

2. Missing Error Handling in Teardown (tests/conftest.py:399-404)

If docker compose down fails, orphaned containers may remain running. The current error handling just prints and continues.

Recommendation:

• Add retries for docker compose down
• Log stderr for debugging
• Consider more robust error handling

3. Platform-Specific Code (tests/conftest.py:372)

preexec_fn=os.setsid is Unix-only. While system tests already skip Windows, this deserves a comment explaining the platform limitation.

🟡 Medium Priority Issues

4. Type Checking (tests/conftest.py:333)

elif type(docker_services) is str:

Should use isinstance() instead:

elif isinstance(docker_services, str):

5. Missing Path Validation (tests/conftest.py:422-431)

EXAMPLE_SERVICES_PATH isn't validated - if it doesn't contain compose.yaml, failures will be silent and confusing.

Recommendation:

example_services_path = os.environ.get("EXAMPLE_SERVICES_PATH")
if example_services_path:
    compose_file = os.path.join(example_services_path, "compose.yaml")
    if not os.path.exists(compose_file):
        raise FileNotFoundError(f"Expected compose file not found: {compose_file}")
    yield from docker_composer(...)

6. Timeout Increase (test_adsim_system.py:37)

TIMEOUT increased from 10s to 60s (6x increase). Is this necessary based on profiling? Could this mask real performance issues? Consider being more specific in the comment about why 60s is needed.

7. Missing Docstrings

The bl01t_di_cam_01 fixture depends on ca_gateway for startup order, but this isn't documented. Consider adding docstrings to key fixtures explaining their dependencies and purpose.

🔵 Minor Suggestions

• Line 379-390: Add flush=True to print statements for better debugging visibility
• Line 422: Simplify: if example_services_path: instead of if example_services_path is not None:
• Dockerfile: Document why docker-28.5.1 and compose v2.40.3 were chosen
• devcontainer.json: Document why --security-opt=label=disable is necessary
• Event loop fixture: Should this be in root conftest.py instead of just system_tests?
• CA cleanup fixture: Should this apply to all EPICS system tests, not just adsim?

📝 Documentation Suggestions

Consider adding documentation for:

• How to run system tests locally in devcontainer
• How to use the manual startup script
• Troubleshooting common issues (docker socket permissions, etc.)

🚀 Recommendation

Approve with minor changes

Please address the critical issues (1, 2, 5) before merging. The medium priority issues would be
good to fix but aren't blockers.

Excellent work on this refactoring! The unified approach is much cleaner than the previous CI-specific setup. 👏