Document differences between CI scripts and git hooks#161
Conversation
2-Coatl
deleted the
feature/investigate-failing-github-actions-09-53-49
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
| result = _run_script(HEALTH_CHECK) | ||
|
|
||
| assert result.returncode in (0, 2), "Health check should pass or skip when Django is unavailable" | ||
| combined_output = f"{result.stdout}\n{result.stderr}" | ||
| assert "Skipping Django checks" in combined_output |
There was a problem hiding this comment.
Lines 36‑40 assert that health-check.sh must print “Skipping Django checks” every time it is executed, so the test fails whenever Django is actually installed and the script successfully runs the database/config validations. That makes the new regression suite unusable for anyone who has a working Django environment, even though the script itself behaves correctly in that scenario. The test should simulate a missing dependency (e.g., by manipulating PYTHONPATH or running in a temporary directory) or accept the success path instead of hard‑coding the degraded behaviour.
Useful? React with 👍 / 👎.
| result = _run_script(BANDIT_SCAN, timeout=5) | ||
|
|
||
| assert result.returncode == 2, "Bandit scan should degrade to SKIP without attempting long installations" | ||
| combined_output = f"{result.stdout}\n{result.stderr}" | ||
| assert "Bandit CLI not detected" in combined_output or "Bandit not installed" in combined_output |
There was a problem hiding this comment.
The expectation in lines 50‑54 is that running bandit-scan.sh must always exit with code 2 and print “Bandit CLI not detected”. As soon as a developer installs Bandit (which is required to actually run the scan), the script exits 0 after running the tool or simply takes longer than the fixed 5‑second timeout, and the test fails even though the scan succeeded. This makes the Python test suite impossible to run on a fully provisioned environment. Please stub Bandit out or adjust the test so it passes when the CLI is present.
Useful? React with 👍 / 👎.
| env = os.environ.copy() | ||
| env["PATH"] = ":".join( | ||
| [ | ||
| "/usr/local/sbin", | ||
| "/usr/local/bin", |
There was a problem hiding this comment.
In lines 58‑62 the test attempts to force a SKIP by rewriting PATH, but it rewrites it to the standard system directories (/usr/local/bin, /usr/bin, etc.), i.e., the very places where npm normally lives. Consequently the CLI is still found whenever Node is installed and the assertions at lines 70‑75 (returncode == 2, “npm CLI not available”) start failing even though the audit ran correctly. To keep the test deterministic you need to stub npm (e.g., clear PATH or point it at an empty temp dir) instead of relying on npm being missing on the host.
Useful? React with 👍 / 👎.
| result = _run_script(TEST_EXECUTION_TIME, timeout=20) | ||
|
|
||
| assert result.returncode == 2, "Test execution validation should skip when Django is absent" | ||
| combined_output = f"{result.stdout}\n{result.stderr}" | ||
| assert "Skipping test execution time validation" in combined_output |
There was a problem hiding this comment.
Lines 79‑83 force test_execution_time.sh to exit with code 2 and to print “Skipping test execution time validation”. That only happens when Django or pytest is missing; as soon as a developer installs the project dependencies the script runs the tests and exits 0/1, so this assertion starts failing even though the validation worked. The regression suite therefore cannot be run in the “happy path” environment it is supposed to guard. Please make the test explicitly simulate the missing dependency or accept the real execution path instead of demanding a SKIP.
Useful? React with 👍 / 👎.
1 participant
Summary
scripts/ci/versusscripts/git-hooks/Testing
Codex Task