./iw ./test is green on code the pre-push hook rejects #466

New issue

Closed

opened 2026-08-27 08:31:18 +00:00 by mph · 1 comment

mph commented

2026-08-27 08:31:18 +00:00

Owner

Found triaging IW-437.

.iw/commands/test.scala runs core.test, dashboard.test and the BATS suites. It never runs scalafix. The pre-push hook does run scalafix over core/, tests included, and .scalafix.conf enables DisableSyntax with noVars.

So both ./mill core.test and ./iw ./test can be fully green on code that cannot be pushed. The phase workflow's verification step calls only those two, which means the failure surfaces at the push - the point where the least context is available and the output is worst (see #465).

OBSERVED. IW-437 phase 1 reached the push gate with two var committedBeforeMerge declarations in PhasePrHarnessTest.scala and a private var in FakeCommandEnv.scala. Every test command was green. The push was blocked.

PROPOSED FIX. Add scalafix to the ./test command so the pre-PR check matches the push gate and green means pushable.

NOTE. This is deliberately a project-level fix, not a change to any shared review guide. Lint configuration differs between projects, and var is not forbidden here in principle - it can be used where it is safe and convenient, either by rewriting or by annotating the use so scalafix accepts it. A shared guide should not carry this project's rules.

Found triaging IW-437. .iw/commands/test.scala runs core.test, dashboard.test and the BATS suites. It never runs scalafix. The pre-push hook does run scalafix over core/, tests included, and .scalafix.conf enables DisableSyntax with noVars. So both ./mill core.test and ./iw ./test can be fully green on code that cannot be pushed. The phase workflow's verification step calls only those two, which means the failure surfaces at the push - the point where the least context is available and the output is worst (see #465). OBSERVED. IW-437 phase 1 reached the push gate with two `var committedBeforeMerge` declarations in PhasePrHarnessTest.scala and a private var in FakeCommandEnv.scala. Every test command was green. The push was blocked. PROPOSED FIX. Add scalafix to the ./test command so the pre-PR check matches the push gate and green means pushable. NOTE. This is deliberately a project-level fix, not a change to any shared review guide. Lint configuration differs between projects, and `var` is not forbidden here in principle - it can be used where it is safe and convenient, either by rewriting or by annotating the use so scalafix accepts it. A shared guide should not carry this project's rules.

mph referenced this issue

2026-09-01 14:02:50 +00:00

tmux-dependent BATS tests fail non-deterministically under a full ./iw ./test run #468

mph referenced this issue

2026-09-01 15:33:00 +00:00

Hooks and CI read a crashed tool as a finding, and print --no-verify as the remedy #497

mph referenced this issue from a commit

2026-09-02 06:22:12 +00:00

test(lint): run the push gate's scalafix check from iw test

mph referenced this issue

2026-09-02 06:22:57 +00:00

Squash the batch-implement blockers: nine fixes for the phase gate, the CI gate and the review-state card #501

mph commented

2026-09-02 20:55:14 +00:00

Author

Owner

Fixed in PR #501 (merge d1eba9c4). ./iw ./test lint runs the exact scalafix command the pre-push hook runs, and is part of the default all-run. It hard-fails when scalafix is absent rather than skipping, because a skip re-creates the false green this issue is about. Green now means pushable.

Fixed in PR #501 (merge d1eba9c4). `./iw ./test lint` runs the exact scalafix command the pre-push hook runs, and is part of the default all-run. It hard-fails when scalafix is absent rather than skipping, because a skip re-creates the false green this issue is about. Green now means pushable.

mph closed this issue

2026-09-02 20:55:14 +00:00