test: assertions positioned before the call under test are a fourth vacuity mode the guard does not detect #176

Found by CodeRabbit on PR LeanerCloud/cloud-commitments-cli#1750, which fixed the one live instance. Filed because the detection is not cheap and PR LeanerCloud/cloud-commitments-cli#1750's claim has been narrowed to match.

The mode

A mock assertion placed before the call under test passes for any implementation:

// vacuous: the mock has recorded nothing yet
mockSP.AssertNotCalled(t, "DescribeSavingsPlansOfferings", mock.Anything, mock.Anything)

id, err := client.findOfferingID(ctx, rec, "test-exec")

This is independent of matcher arity, so it survives every repair LeanerCloud/cloud-commitments-cli#1595 and LeanerCloud/cloud-commitments-cli#1740 made. It is a fourth structural cause alongside:

1. a call served from a default that never reaches mock.Called (fix(test): MockConfigStore isExpected short-circuit makes 20 AssertNotCalled assertions unfailable cloud-commitments-cli#1595, MockConfigStore only)
2. a name-only assertion against a parameterised method (fix(test): name-only AssertNotCalled is vacuous on every mock type, not just MockConfigStore cloud-commitments-cli#1740)
3. a non-zero matcher count that differs from the real arity (fix(test): make 78 unfailable MockConfigStore assertions able to fail cloud-commitments-cli#1735)
4. an assertion positioned before the call under test — this issue

Demonstrated on the one live instance, with the call tolerated so the assertion rather than testify's panic decides:

Why TestNoUnfailableMockAssertions does not catch it

The guard compares matcher count against the method's m.Called arity. It has no notion of "the call under test", and inferring one is the hard part — any call in the test could be it, or none could.

A prototype detector was written and run: for each assertion, report any later call in the same scope that is not itself an assertion or test scaffolding. Refinements applied: innermost-scope resolution so sibling t.Run subtests do not count, and skipping t.Cleanup bodies since they execute after the test returns.

Result across the files PR LeanerCloud/cloud-commitments-cli#1750 touched: 10 findings, 1 true positive, 9 false positives. The false positives are calls appearing as arguments to other assertions (assert.False(t, scheduler.collecting.Load())) and assertions inside subtests whose enclosing scope contains later unrelated calls.

A check at that ratio is worse than no check — it trains readers to ignore it, and a guard that is ignored gets deleted. That is the same failure direction the whole LeanerCloud/cloud-commitments-cli#1595/#1740 line of work exists to avoid, so it was not merged into the guard.

What would make it tractable

Some combination of:

• Requiring the assertion's mock to be passed into something called later in the same scope, rather than treating any later call as the call under test.
• Restricting to AssertNotCalled on a mock with no recorded interaction anywhere earlier in the scope — narrower, far fewer false positives, and it would have caught the live instance.
• Treating it as a go vet-style analyzer with errors.As-grade type information rather than a syntactic pass.

The second option looks most promising and may be cheap. It is not attempted here because LeanerCloud/cloud-commitments-cli#1750 was already blocked on three other findings.

Meanwhile

Reviewers should check ordering by eye on any AssertNotCalled. The rule is simply: the assertion belongs after the code under test, and a negative assertion sitting above the call is always wrong.

Related

• fix(test): name-only AssertNotCalled is vacuous on every mock type, not just MockConfigStore cloud-commitments-cli#1740 / PR fix(test): repair 30 unfailable mock assertions and guard the class cloud-commitments-cli#1750 — where this surfaced; fixes the one live instance and narrows its own claim to the matcher-count modes.
• fix(test): MockConfigStore isExpected short-circuit makes 20 AssertNotCalled assertions unfailable cloud-commitments-cli#1595 / PR fix(test): make 78 unfailable MockConfigStore assertions able to fail cloud-commitments-cli#1735 — causes (a) and (c).
• The verification method used in PR fix(test): name-only AssertNotCalled is vacuous on every mock type, not just MockConfigStore cloud-commitments-cli#1740 (injecting a recorded call immediately before the assertion) is blind to this mode by construction: it places the call before the assertion, which is exactly the ordering under test. Any future verification of assertion-failability should exercise the real call path rather than a synthetic one where that matters.

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A15-004 (medium)

A second gap in the same guard, structural rather than a new vacuity mode. TestNoUnfailableMockAssertions walks the repo and then per directory does mocks := collectMocks(files); if len(mocks) == 0 { continue } (internal/mocks/vacuous_assertion_guard_test.go:137). A package that uses a mock imported from elsewhere but declares none locally is dropped before its assertion sites are collected, and it is not added to the skipped slice the test errors on -- which contradicts the guard's own contract that a silent skip is the defect it exists to catch. Replaying the walk over the pinned tree, four packages with 15 assertion sites are invisible: internal/server (6), providers/azure/services/compute (4), managedredis (3), synapse (2), while the run logs "checked 162 mock assertion site(s)" and passes with zero skips reported. None of the 15 is vacuous today (each passes a matcher count equal to the real m.Called arity), so this is a hole rather than a live unfailable assertion. Collecting assertion sites before the len(mocks) == 0 test and resolving them through the existing repo-wide global index, routing anything unresolved into skipped, closes it. (audit finding A15-004)

A16-012 (low)

Another shape for this catalogue, and it has live instances. In internal/auth/account_scope_fail_closed_test.go:74, assert.False(t, IsUnrestrictedAccess(got) && err == nil, ...) sits after require.Error(t, err) at :69, which aborts the subtest when err == nil. By line 74 err != nil holds unconditionally, so the && err == nil conjunct is constant false and the assertion cannot fire for any value of IsUnrestrictedAccess(got). The comment above it claims it restates the security property independently of the preceding assertions, which is the risk: a reader auditing the file sees the invariant explicitly asserted and stops looking, when the real protection is assert.Nil(t, got) on the line above. The construction repeats verbatim at :192 (with its require.Error at :187). Dropping the && err == nil conjunct makes both assertions say what their comment claims. The general shape -- a boolean conjunct that a preceding require has already fixed -- looks statically detectable in the same way the other modes are. (audit finding A16-012)