Skip to content

Neither gate compiles the test suite, so test code that cannot compile passes green #771

Description

@timujinne

Both mix precommit and .githooks/pre-commit are structurally unable to notice test code that does not compile (as opposed to test code that is badly formatted or wrongly typed). Two independent mechanisms produce the same blind spot: a tree whose test suite cannot compile finishes the gate with exit code 0.

How we hit it

Merging main (3d6c941d52) into our fork produced a test file with two identical describe blocks in test/phoenix_kit/install/oban_config_test.exs:

line 702:  describe "ensure_pruner_max_age/2 — I103: exposed for testing, previously untested"
line 770:  describe "ensure_pruner_max_age/2 — I103: exposed for testing, previously untested"

Neither side had a duplicate: our parent had 8 describe blocks, yours had 10, the merge had 11. Git reported no conflict, because the two insertions never overlapped line-wise.

To be exact about whose tree this was: it was our merge, not a commit in your history. We amended it away the same night, and it is now unreachable even here. main is clean — we checked every unique blob of that file across all refs, and every *_test.exs at 3d6c941d52: no duplicates anywhere. We are reporting it because the gate did not catch it, and merging is something you do far more often than we do.

On that tree:

mix format             clean
mix credo --strict     11829 mods/funs, found no issues
mix dialyzer           passed
gate exit code         0

Why a duplicate describe is a compile error

It is raised while the macro expands, not while tests run:

** (ArgumentError) describe "foo/1" is already defined in PATest
    (ex_unit 1.18.4) lib/ex_unit/callbacks.ex:767: ExUnit.Callbacks.__describe__/4

Nine lines reproduce it on bare Elixir 1.18.4 — no project, no database, no test executed. Changing only the name in the second describe makes the same file exit 0.

Mechanism A — mix precommit never compiles .exs at all

elixirc_paths(:test) is ["lib", "test/support"] (mix.exs:73), and compile.elixir compiles only files with the .ex extension. Tests are .exs, so no environment compiles them, wherever they sit. format checks syntax only, and two identical describe blocks are valid syntax. credo parses the AST without expanding macros — and describe is the macro that raises. dialyzer works from the lib beams.

Visible in the build artifacts: _build/test/lib/phoenix_kit/ebin holds 0 *Test.beam files.

Mechanism B — the git hook compiles under :dev

.githooks/pre-commit:189 runs mix compile --warnings-as-errors --all-warnings, and MIX_ENV appears nowhere in the script (grep -c MIX_ENV .githooks/pre-commit → 0). So it compiles under the default :dev, where elixirc_paths(_) is ["lib"] — even test/support/*.ex, ordinary .ex files a plain compiler could see, are outside the path set for that environment.

Visible in the build artifacts: all seven test/support modules have a .beam under _build/test and none under _build/dev.

A deliberately broken test/support/*.ex confirms it: the hook's exact command exits 0 and prints nothing; the same file under MIX_ENV=test exits 1 with a MismatchedDelimiterError. The failure path exists and works — the hook simply walks past it by construction.

This is not the mix test step you already decided against

mix.exs:366-373 records why mix test is deliberately kept out of precommit: without a database some "unit" tests fail, with one the unbounded concurrency exhausts the pool, and a gate that is always red gets ignored. All three reasons are about running tests. A step that only compiles the test tree runs none — it does not require test_helper.exs, so ExUnit.start never happens, no database is touched, no migration runs. Measured incidentally: in the environment where we timed the step, the database was in fact unreachable (psql -lqtpassword authentication failed, the very check test_helper.exs makes), and the step still passed cleanly.

mix.exs already contains a step shaped exactly this way: "test.js": &run_js_tests/1 skips itself when node isn't installed, "rather than failing a contributor's precommit over an optional tool".

If you add such a step, it has to fail loudly

Kernel.ParallelCompiler.compile/2 does not raise on a compile error — it returns {:error, errors, warnings} and carries on. (There is no compile!/2 in Elixir 1.18; function_exported?(Kernel.ParallelCompiler, :compile!, 2)false.) A naive implementation prints == Compilation error == and still exits 0 — reproducing the exact defect it was added to fix. The result must be matched explicitly and a non-zero exit forced.

Cost

Measured on our machine, paired (gate with the step against without), twice, under disclosed competing load: Δreal 43.7 s and 62.6 s. It does not add minutes, but the second run came close to one, and we would rather say that than round it down. For comparison, on a warm tree credo --strict plus dialyzer — the path a broken tree currently walks all the way to green — costs about 3.5 minutes here (credo 64.4 s, PLT build 49.8 s, analysis 1 m 35 s). Your timings will differ; the proportion is the point.


Environment: Elixir 1.18.4 / OTP 28. Timings are ours and were not reproduced on your hardware; every other figure above was produced by running, not by reading.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions