Restore strict shell flags without losing CI summary#159
Restore strict shell flags without losing CI summary#159NestorMonroy merged 2 commits intodevelopfrom
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.
Remove assumption that run-all-checks fails
The new test test_run_all_checks_reports_summary_even_on_failure now hard-codes assert result.returncode != 0 when invoking scripts/ci/run-all-checks.sh. That means the Python test suite will always fail in any environment where the CI suite actually passes (which is the desired steady state once dependencies are installed). Nothing in the test enforces or simulates failure—it simply assumes the host lacks prerequisites. As soon as someone runs the suite in a configured environment (or once CI is fixed), the entire test module will fail even though the code under test works, blocking merges.
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 expects failure-specific output
test_health_check_surfaces_underlying_error_details likewise asserts that running scripts/ci/infrastructure/health-check.sh must exit non‑zero and emit ModuleNotFoundError/No module named. This bakes in a very specific failure mode (missing Python deps) and causes the test suite to fail whenever the health check actually succeeds or fails for a different reason (e.g., DB connectivity). Once the environment is healthy, this test will fail 100% of the time even though the code is correct, so the suite cannot pass.
Useful? React with 👍 / 👎.
Summary
set -euo pipefailinrun-all-checks.shand wrap each suite execution so failures surface in the final report without aborting the pipelineinvoke_checkhelper to keep fail-fast semantics while allowing aggregated reporting under strict modeTesting
Codex Task