skip to content

Why is a conditional or loop inside a test body a smell, and what replaces it?

level: middleimportance: should knowfreq 46%

answer

  1. Nothing verifies the verifier
  2. The characteristic bug is a silent pass
  3. A loop body can run zero times
  4. Two tests instead of one branch
  5. Skipped is reported; early return is not

basics

~20 s

Nothing verifies test code, so a branch or loop in a test adds an unchecked path that can report success while checking nothing. Replace a branch with two named tests, and a loop with case-per-input reporting or a whole-value assertion.

solid answer

~50 s

A test should contain no decisions: no branch, no early-return guard, no loop wrapped around assertions, no exception handling used as control flow. The reason is that the test is the oracle and nothing tests the oracle, so a bug in that logic does not make the suite red — it makes the suite lie. The characteristic failure is a silent pass: a loop asserting each element of a returned collection runs zero times when the collection comes back empty, and the test stays green while the data is corrupt. The replacements are specific. A branch becomes two tests, each arranging its own condition and named for its case. A loop over inputs becomes the runner's data-driven case-per-input facility, so every input is reported separately. A loop over results becomes one whole-value comparison. An environment check becomes the runner's skip mechanism, which reports a hole instead of a pass.

code

pseudocode · 6 lines
pseudocode
test "published slots keep their allocated room":
    slots = planner.publish(term).slots
    for slot in slots:
        assertEquals(allocation.roomFor(slot), slot.room)
    // slots came back empty after a mapping change:
    // zero assertions ran, the test passed, rooms shipped blank

go deeper

for a junior

Be ready to say that a test should contain no branches or loops around its checks, and to give the concrete danger: a loop over an empty collection checks nothing and still reports success.

for a middle

Explain the mechanics of each shape — vacuous loop, unreported branch arm, early-return guard, swallowed exception — and name the specific replacement for each, including case-per-input reporting rather than a hand-written loop.

for a senior

Show the judgement about arrange-phase loops being acceptable construction, and be able to describe how a silent pass survived in a real suite for weeks and what signal would have caught it earlier.

for a principal

Own the policy question: whether to enforce complexity limits on test sources, what that gate costs teams in practice, and how you keep the standard as a shared understanding of oracle risk rather than a rule people route around.

### The rule and the reason A test body should contain **no decisions**. No `if`/`else`, no `switch`, no early return guard, no loop wrapped around assertions, no `try`/`catch` used as control flow. The reason is a single sentence that is worth memorising: **the test is the oracle, and nothing tests the oracle.** Production code is protected by tests; test code is protected by nothing. Every branch you add to a test adds a path that could be wrong in a way that reports "passed". That is why this smell is graded more harshly than the same construct in production code. A bug in production code makes the system wrong and the suite goes red. A bug in a test makes the suite *lie*, and a lying suite is worse than no suite, because the team stops looking. ### The four shapes, and what each one hides **A loop around assertions can assert nothing at all.** A timetable planner test publishes a term schedule, then loops over the returned slots asserting that each one's room matches its allocation. A mapping change started writing empty room references, the publish call returned an empty slot list, the loop body ran zero times, and the test passed. The corrupted timetable stayed green through forty-one builds over nine days before anyone noticed rooms were blank. A loop with no assertion on the collection itself is a **vacuous pass**: it verifies the elements it happens to be given, including none of them. The fix is to assert the collection as a whole — its size, or better, a comparison of the whole collection against an expected collection in one assertion. A structural comparison of the whole value also reports every difference at once rather than stopping at the first element that differs. **A branch means different runs test different things.** `if (planner supports split terms) assert A else assert B` produces a green result that does not say which arm ran. Six months later the condition is permanently false in the build environment and half the test has quietly stopped existing. The fix is two tests, each of which *arranges* its condition explicitly and is named for the case it covers, so the report shows two results and a missing one is visible. **A guard clause skips the verification and calls it success.** `if (results are empty) return` at the top of a test is a decision to pass when the interesting case did not occur. If the empty case is legitimate, assert it; if it is not, fail loudly. **A `try`/`catch` around the act step swallows the outcome.** Catching the expected exception and asserting inside the catch block passes when no exception is thrown at all — the block simply never runs. Use the framework's assert-throws construct, which fails when nothing is thrown. ### What to write instead - **Two tests instead of a branch**, each named for its case. - **Case-per-input reporting instead of a loop over inputs.** When the same assertions must run across many inputs, use the runner's data-driven facility so each input is reported as its own result. A hand-written loop stops at the first failure, hides the remaining cases, and names none of them in the report. - **A whole-value assertion instead of an element loop.** - **The runner's skip/assumption mechanism instead of an environment `if`.** A skipped test is reported as skipped; an `if` that returns early is reported as passed. The difference is whether the team can see the hole. ### The negotiable case: loops in the arrange phase Not all iteration is a decision. Building forty timetable slots, or the traffic fixture for a publish endpoint that peaks around 1,200 requests per minute, is **data construction**, and a loop there does not create an unverified path through the verification. It is still worth extracting into a named builder so the test body reads declaratively, but it is a readability preference, not the smell. The sharp version of the rule is therefore: *no decisions anywhere in the test; construction loops belong in a helper, not in the test body.* ### Detecting and discussing it Cyclomatic complexity of one, per test method, is a cheap and surprisingly effective standard: most static analysers can be pointed at test sources with that threshold. The value in an interview is less the rule than the failure mode you can name — a conditional test's characteristic bug is a **silent pass**, not a wrong failure, and that is what makes it a smell rather than a style preference. A candidate who says "loops in tests are ugly" has the opinion; a candidate who says "the loop body ran zero times and the suite went green" has the reason.

  • Is a loop in the arrange phase the same smell?
    No. Building forty timetable slots, or a traffic fixture sized for a peak of about 1,200 requests per minute, is data construction, not a decision about what to verify, so it adds no unverified path through the checking. It is still worth extracting into a named builder so the test body reads declaratively, but that is a readability preference rather than the smell.
  • A test only makes sense on one operating system. How do you express that without a branch?
    Use the runner's skip or assumption mechanism, so the result is reported as skipped. An early return reports the test as passed, which means the coverage hole is invisible: nobody can tell from the report that the behaviour was never checked on this machine. Skipped results also show up in trend reports when a whole platform quietly stops running.
  • How would you catch this smell automatically?
    Point a static analyser at the test sources with a cyclomatic-complexity limit of one per test method. It is cheap and catches branches, guards and loops in one rule. Pair it with a check that flags tests containing no assertion, since the vacuous-pass failure often ends up there too.

A test is a measuring instrument. Putting an if-statement inside it is like wiring a thermometer so that it sometimes reports the room temperature and sometimes reports nothing at all, while the dial still reads 'normal'.

saying these in an interview costs you the question

  • Says loops in tests are merely untidy, with no failure mode
  • Uses an early return to skip a test and calls it passing
  • Asserts inside a catch block without failing when nothing is thrown
  • Writes one loop over inputs instead of case-per-input reporting
  • Believes test code needs less scrutiny than production code
  • Adds a branch so one test covers two configurations

context

resolving links…