Fix CI shell orchestrator resilience#158
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| def test_run_all_checks_reports_summary_even_on_failure(): | ||
| result = _run_script(RUN_ALL_CHECKS) | ||
|
|
||
| assert result.returncode != 0, "Expected the aggregated checks to fail in the default dev environment" | ||
| combined_output = f"{result.stdout}\\n{result.stderr}" | ||
| assert "FINAL CI/CD REPORT" in combined_output |
There was a problem hiding this comment.
Test wrongly expects run-all-checks to fail
Lines 19‑24 make test_run_all_checks_reports_summary_even_on_failure assert that run-all-checks.sh exits with a non‑zero status in the default environment. The CI orchestrator is supposed to exit 0 when all checks pass, so any developer or CI runner that happens to have the required tooling installed will see this test fail even though the script is behaving correctly. The test should assert that the report is printed regardless of whether the run succeeds or fails, not hardcode failure as the expected outcome.
Useful? React with 👍 / 👎.
| def test_health_check_surfaces_underlying_error_details(): | ||
| result = _run_script(HEALTH_CHECK) | ||
|
|
||
| assert result.returncode != 0, "The health check should fail when dependencies are missing" | ||
| combined_output = f"{result.stdout}\\n{result.stderr}" | ||
| assert "ModuleNotFoundError" in combined_output or "No module named" in combined_output |
There was a problem hiding this comment.
Health check test depends on missing dependencies
Lines 27‑32 force test_health_check_surfaces_underlying_error_details to expect the health check script to fail with a ModuleNotFoundError. On a machine where the Django project dependencies are actually installed, the health check will succeed and no such text will appear, so the unit test fails despite the environment being correctly configured. This makes the test suite inherently flaky and will break as soon as CI gets a full dependency set. The test should not rely on the absence of dependencies to pass; it needs a deterministic fixture instead.
Useful? React with 👍 / 👎.
Summary
scripts/ci/run-all-checks.shfrom exiting on the first failure and surface failing script output to the userTesting
Codex Task