Skip to content

tests: make the dash-loop tests actually run (CI red since 15 Sep) - #64

Open
ThinkOffApp wants to merge 1 commit into
mainfrom
fix/ci-dash-loop-never-ran
Open

ThinkOffApp wants to merge 1 commit into
mainfrom
fix/ci-dash-loop-never-ran

Conversation

@ThinkOffApp

Copy link
Copy Markdown
Owner

CI has been red on main for three days

Every run since 15 Sep fails identically:

ModuleNotFoundError: No module named 'pytest'
Ran 115 tests in 8.137s
FAILED (errors=1)

tests/test_dash_loop.py imports pytest for two @pytest.mark.skipif decorators. pytest is not installed on the runner, and the workflow runs python3 -m unittest discover, so the import fails before anything runs.

The worse half

These are bare def test_*(tmp_path) functions. unittest never collects those, so even with pytest installed they would not have run under the command CI actually uses.

The dash-loop regressions from #57 / #59 / #61 have never once executed in CI — a stalled poll killing the scheduler, and F's own timeout being overwritten by the caller's options. The only thing this file has ever reported is its own import error. Three PRs merged into that state with no test signal.

Change

unittest.TestCase with unittest.skipUnless(shutil.which("node")), and the pytest tmp_path fixture replaced with tempfile. No test logic altered — the assertions are the same ones codexmb's reviews asked for.

before   Ran 115 tests   FAILED (errors=1)   dash-loop: never executed
after    Ran 121 tests   OK                  dash-loop: 2 running, both pass

A permanently red CI is as useless as a green one that checks nothing: nobody can see a real break in it.

Unblocks the signal on #63.

🤖 Generated with Claude Code

CI has failed on main for three days, every run the same way:

    ModuleNotFoundError: No module named 'pytest'
    Ran 115 tests ... FAILED (errors=1)

test_dash_loop.py imported pytest for two `@pytest.mark.skipif` decorators.
pytest is not installed on the runner, and the workflow runs
`python3 -m unittest discover`, so the import blew up before anything ran.

Worse than a red tick: these are bare `def test_*` functions, which unittest
never collects. Even with pytest installed they would not have run under the
command CI actually uses. The dash-loop regressions from #57/#59/#61 - a
stalled poll killing the scheduler, and F's own timeout being overwritten by
the caller's options - have never once executed in CI. The only thing this
file ever reported was its own import error.

Converted to a unittest.TestCase with unittest.skipUnless(node), and the
pytest `tmp_path` fixture replaced with tempfile. Both tests now run and pass.

    before   Ran 115 tests   FAILED (errors=1)   dash-loop: never executed
    after    Ran 121 tests   OK                  dash-loop: 2 running

A permanently red CI is as useless as a green one that checks nothing: nobody
can see a real break in it. Three PRs merged into that state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Sep 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
carwatch-dev Ready Ready Preview Sep 18, 2026 4:30pm UTC

Request Review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-18T16:33:12.280265Z 8429f75 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8429f75755

ℹ️ 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".

Comment thread tests/test_dash_loop.py
def test_fetch_helper_abort_reaches_the_request(self):
"""codexmb, #61: Object.assign({signal}, o) let the caller's options
overwrite the helper's own signal, so its timeout never aborted."""
r = _run_node(F_HARNESS, _fetch_helper_js())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cancel the harness's ten-second timer

When Node is available, this newly enabled test keeps every test run alive for roughly ten extra seconds: the third F_HARNESS case calls F(..., 10000), while its stubbed fetch never settles or rejects on abort, so F cannot clear its timeout after the outer abort assertion completes. Running this test alone took 10.29 seconds versus 0.20 seconds for the scheduler test; use a short timeout or explicitly clean up the outstanding timer so the conversion does not impose this delay on every CI run.

Useful? React with 👍 / 👎.

@ThinkOffApp

Copy link
Copy Markdown
Owner Author

Merge order verified — you can take these one at a time in any order.

I had only tested the seven branches merged together into a scratch branch, which is not how they will actually land. Re-tested the realistic sequence: this PR first, then each of the others onto the accumulating main.

#64  ci-dash-loop-never-ran          merged
#65  no-data-post-on-every-boot      OK
#63  tyre-pressure-units             OK
#66  grounding-car-vs-computer       OK
#67  answers-leak-the-scaffold       OK
#68  status-wildcard-cors            OK
#69  elm327-silent-swallows          OK

full suite after the whole sequence:  Ran 148 tests  OK

No conflicts and no rebases needed, even though three of them touch agent.py and two touch grounding.py.

This one first still makes sense — until it lands, the other six show a red tick that means nothing, because test_dash_loop fails on main for a missing pytest. It is also the only one of the seven that cannot affect the car: it touches tests/ and nothing the Pi runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants