test(proxy): make the dangling-marker assertion baseline-relative, not absolute - #203
Merged
Conversation
…t absolute #188 added a third instance of the defect #192 documents, and it is mine. TestAnExhaustedRewindReserveNeitherStampsUnbackedMarkersNorHidesItself asserted `snap.StashMissing != 0` as an ABSOLUTE value. StashMissing is a process-wide counter shared with every test in the binary, and TestStatsPublishesDanglingReplaysFromTheLiveCounter — added in the same PR, declared below it in the same file — deliberately drives it non-zero. So the assertion passed on the first run and failed on the second: go test ./proxy/ -run "TestAnExhaustedRewindReserve|TestStatsPublishesDangling" -count=2 --- FAIL: TestAnExhaustedRewindReserveNeitherStampsUnbackedMarkersNorHidesItself /stats stash_missing = 1 in a fixture where every refusal left the content verbatim; a declined removal is being counted as a dangling marker The message was even accurate about its own confusion: on the second run a DIFFERENT test's dangling marker was being read as this fixture's. Now baseline-relative — captured before the request this fixture makes, so the assertion measures what the fixture did rather than what the binary has accumulated. Other tests in the same file were already written this way (StashRefusals, and the live-counter comparison below); this one was not, which is the whole of the defect. Verification: gofmt -l . clean, go vet ./... clean, go test ./... all packages pass, and the two tests now pass at -count=2. Revert-verified: restoring the absolute form fails at -count=2 with /stats stash_missing moved by 1 in a fixture where every refusal left the content verbatim The first cut of that mutation did not compile — reverting the assertion left `missingBefore` unused — so it was re-cut with `_ = missingBefore` to fail as a TEST rather than as a build error. A mutation that breaks the build is not a caught mutation. #192's count is accurate again: at -count=2 the proxy package now fails only on TestExtractEconomicsAreExported and TestExpandUnresolvedSeriesRender, which are the two pre-existing instances that issue describes. Signed-off-by: DAVID AMID <DAVIDA@il.ibm.com> Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #192.
What this fixes
#188 added a third instance of the defect #192 documents, and it is mine.
TestAnExhaustedRewindReserveNeitherStampsUnbackedMarkersNorHidesItselfassertedsnap.StashMissing != 0as an absolute value.StashMissingis a process-wide counter shared with every test in the binary, andTestStatsPublishesDanglingReplaysFromTheLiveCounter— added in the same PR, declared below it in the same file — deliberately drives it non-zero.So it passed on the first run and failed on the second:
The failure message was accidentally accurate about its own confusion: on the second run it was reading a different test's dangling marker as this fixture's.
The fix
Baseline-relative — captured before the request this fixture makes, so the assertion measures what the fixture did rather than what the binary has accumulated.
Worth noting the other assertions in the same file were already written this way (the
StashRefusalsdelta, and the live-counter comparison in the test below). This one was not, which is the whole of the defect — an inconsistency inside one file rather than a missing convention.Verification
gofmt -l .clean ·go vet ./...clean ·go test ./...all packages pass · the two tests pass at-count=2.Revert-verified. Restoring the absolute form fails at
-count=2:The first cut of that mutation did not compile — reverting the assertion left
missingBeforeunused — so it was re-cut with_ = missingBeforeto fail as a test rather than as a build error. A mutation that breaks the build is not a caught mutation.Effect on #192
Its count is accurate again. At
-count=2the proxy package now fails only onTestExtractEconomicsAreExportedandTestExpandUnresolvedSeriesRender— the two pre-existing instances that issue describes.How it was found
Verifying, after #188 merged, which issues its merge should have closed. It closed #187 correctly; this turned up because #192 exists and I had added tests to the package it is about, so the honest check was whether I had made it worse. I had.