fix(tests): move the dev-lead session-output sidecar under DEV_LEAD_SIDECAR_DIR (part 2)

Merged
DO
8d ago
+78-86

Problem

Part 2 of "Test suites share fixed files under /tmp" (#2271). Part 1 (#2296) gave five of the six dev-lead sidecar files a per-test directory. The sixth, dev-lead-session-output.txt, was left on a fixed path because moving it rewrites lines that "fix(tests): make every negated bats assertion able to fail" (#2264) was also rewriting. That has merged.

The session log is the most exposed of the six: test_fix_reviews.bats seeds it and then asserts on its absence, and test_engine_writer.bats and test_engine_fallback.bats assert on its content, so two sessions on one machine overwrite each other's file. It showed up again today: run several suites side by side and test_fix_reviews and test_engine_fallback lose tests that pass alone.

Design

The same single definition as part 1. run_writer writes, and dev-lead-fix-issue.sh and dev-lead-fix-reviews.sh read, "$DEV_LEAD_SIDECAR_DIR/dev-lead-session-output.txt"; with the variable unset that is /tmp/dev-lead-session-output.txt, as before. The suites already export the variable from part 1.

The six "relocation pending" rows are removed from scripts/lib/fixed-tmp-paths.tsv, so the static check now fails if a literal comes back.

After this, the one fixed path under /tmp still shared between tests is /tmp/cascade in scripts/review-one-pr.sh, which needs a change on three sides and is tracked as #2302.

Risk

  • Production: with the variable unset the path is byte for byte what it was. No workflow sets it or reads the file.
  • The conflict with #2264 was resolved by keeping that pull request's || false form on every negated line and changing only the path.
  • From the QA advisory on part 1, not done here: a suite-level default for a suite that forgets the export. "fix(engine): keep the token ledger out of test suites" (#2311) is adding suite hooks to every test directory; the default belongs in those hooks once it merges, to avoid two pull requests editing the same six lines.

Test plan

  • test_engine_writer 43 of 43, test_engine_fallback 38 of 38, test_fix_issue 32 of 32, the fixed-path check's suite 18 of 18.
  • test_fix_reviews: 272 pass and 1 fails. The failure ("#2143: the remote head moved during the test job") fails the same way on unmodified main on this machine, alone and in the full file, so it is not from this change. CI is the authority.
  • scripts/check-fixed-tmp-paths.sh exits 0 with the six rows removed; the negation lint from #2264 exits 0; shellcheck --severity=warning -x clean on the three scripts.
  • From the original work on this change: with a background loop writing garbage to the default paths, test_engine_writer went from 41 passing and 2 failing on main to 43 passing.
  • Not run: CI.

Rollback

Revert the squash commit; the six exception rows return with it.

Monitoring

None beyond CI. A failure in test_check_fixed_tmp_paths.bats after another pull request merges names the file that added a fixed path.

Closes #2271

🤖 Generated with Claude Code