skip to content

A pull request adds a green test asserting reflect.DeepEqual(got, want) on a result struct - why might it prove nothing?

level: seniorimportance: nice to knowfreq 30%

answer

  1. green is not evidence of coverage
  2. ask what would make it fail
  3. where did want come from
  4. both sides zero still matches
  5. break the function and rerun

basics

~20 s

A green whole-struct comparison proves nothing when want came from the same code path, or when both sides are zero values and every field trivially matches. It can also over-constrain, breaking on fields the case never meant to pin.

solid answer

~40 s

Three failure modes hide behind a green `reflect.DeepEqual`. First, **tautology**: if `want` was built by calling the code under test, or by the same helper the function uses, the assertion compares a value with itself and passes whatever the function does. Second, **trivial agreement**: if the call returned a zero-value struct and the test built an empty `want`, every field matches and nothing about behaviour is pinned. Third, **over-constraint**: DeepEqual compares every field, including ones the case never meant to assert, so the test breaks on unrelated changes - and the usual reaction is to regenerate `want` from actual output, which converts it into the first failure mode. The cheap review check is to break the function deliberately and confirm the test goes red. If it stays green, the assertion is decoration.

code

go · 6 lines
go
got := Compute(order)
want := Compute(order) // derived from the code under test

if !reflect.DeepEqual(got, want) {
	t.Errorf("Compute(order) = %#v, want %#v", got, want)
}

go deeper

for a junior

Take away one habit: after writing an assertion, break the function on purpose and confirm the test turns red. A test that cannot fail is not protecting anything.

for a middle

Be able to name the three ways a green whole-struct comparison misleads - a want derived from the code itself, both sides being zero values, and comparing fields the case never meant to pin.

for a senior

Review assertions by asking what would make them fail, and connect over-constraint to tautology: a noisy assertion gets regenerated from actual output, which is how a suite quietly stops testing anything.

for a principal

Set the expectation that expectations are derived independently, and decide what your team does about volatile fields and computed floats before every package invents its own tolerance.

## Green is not evidence A reviewer's job on a test is not to confirm it passes - CI already did that. It is to work out **what would have to be true for it to fail**. A whole-struct `reflect.DeepEqual` assertion is where that question is most often skipped, because the line looks thorough: it compares everything. ## Failure mode one: the tautology ```go got := Compute(order) want := Compute(order) if !reflect.DeepEqual(got, want) { t.Errorf("Compute(order) = %#v, want %#v", got, want) } ``` Written out this baldly it is obvious, but it arrives in disguise. `want` is built by a test helper that calls the same constructor the production path calls. Or the expected total is computed in the test with the same rounding helper the library uses, so a bug in the helper cancels out on both sides. Or `want` was originally produced by printing `got` during development and pasting it back. The test then asserts that the code is self-consistent, which it always is. **An expectation must be derived independently** - written by hand from the specification, or computed by an obviously different method - or it is not an expectation at all. ## Failure mode two: trivial agreement A `Result` struct with a total, a slice of line items and a timestamp. The function under test hits an early return and hands back a zero-value `Result`. The test built `want` by filling in nothing, or by filling in only the field the case is about and leaving the rest zero. Every field matches. The assertion is green and the function did not do its job. The tell is a `want` that is mostly zero values. Ask what the case is about; if the answer is one field, assert that field and say so. ## Failure mode three: over-constraint DeepEqual compares **every** field, including ones the case never intended to pin: a generated id, a `CalculatedAt` timestamp, a cache field, a nested slice whose ordering is incidental. That assertion is red not when behaviour is wrong but whenever anything at all changes, and the team's response to a suite that cries wolf is predictable - someone regenerates `want` from the actual output, and the suite quietly becomes failure mode one. The fix is to make the assertion say what the case means. Compare a reduced projection holding only the fields under test, or zero the volatile fields on both sides before comparing, visibly, so the next reader can see what is deliberately not being asserted. ## Floats make it worse DeepEqual compares `float64` fields with `==`. Two totals that differ by one unit in the last place fail, and the reflexive fix is a tolerance that then hides real errors. `NaN` is its own trap: `NaN == NaN` is false, so two structs whose totals are both `NaN` are **not** deeply equal - a test that produced `NaN` on both sides fails rather than passing, which is at least loud, but it also means you cannot assert "both are NaN" this way. For money in particular the answer is not a tolerance; it is to stop using binary floating point and hold integer minor units or a decimal type, at which point exact comparison is honest again. ## The check that costs thirty seconds **Break the function on purpose and rerun the test.** Change a constant, flip a comparison, return early. If the test still passes, the assertion is decoration and the pull request has not added coverage. This is the entire idea behind mutation testing, applied by hand to one assertion during review, and it catches all three failure modes above without any analysis. A second, cheaper check: read the failure message the assertion would print. If it is `%v` on a large struct, a reader will get a wall of text with no indication of which field differed - so even when the assertion does fail correctly, nobody will be able to act on it quickly. `%#v` or a per-field comparison earns its keep here. ## What to write in the review Not "this test is wrong", but the question that exposes it: **what would break this?** If the author cannot name a change to the function that would turn the test red, the assertion is not pinning behaviour, and the conversation moves to what the case is actually meant to protect.

  • How do you check quickly whether an assertion pins any behaviour at all?
    Break the function deliberately - change a constant, invert a condition, return early - and rerun the test. If it stays green, the assertion is decoration. It takes under a minute during review, it needs no tooling, and it catches tautological expectations, trivially-matching zero values and assertions that were quietly loosened until they always pass.
  • A whole-struct comparison keeps breaking on fields the case does not care about. What do you change?
    Narrow the assertion, visibly. Build a reduced value holding only the fields under test, or zero the volatile ones - generated ids, timestamps - on both sides before comparing, with a comment saying why. What you refuse is the other fix: regenerating `want` by pasting in the actual output, which turns a noisy assertion into one that can never fail.
  • How does a float64 field change the picture?
    reflect.DeepEqual compares float fields with ==, so totals differing in the last bit fail and a tolerance gets bolted on that then hides real errors. NaN never equals itself, so two structs whose totals are both NaN are not deeply equal. For money the answer is not a tolerance but integer minor units or a decimal type, which makes exact comparison honest.

Marking your own exam against a copy of your own answers gives full marks every time, and tells you nothing about whether the answers were right.

saying these in an interview costs you the question

  • Treats a green test as evidence the behaviour is covered
  • Regenerates want by printing the actual output and pasting it back
  • Never checks that the assertion fails when the function is broken
  • Compares computed float64 totals exactly and blames the flake on the test runner
  • Builds want with the same helper the code under test uses