Conversation
…uctural Three tests were executing in CI while being structurally incapable of failing, and the guard meant to prevent exactly that could not see the regression it was built to catch. UpdateOfferDialog.zig asserted `expectEqual(Action.later, .later)` -- an enum compared to itself, invoking no production code at all, under a name claiming to cover "install unavailable". The real contracts lived inside windowProc and createButton, unreachable from a unit test. Extract them into `buttons` and `actionForCommand`, have windowProc consume those instead of open-coding the same decisions, and assert the actual behaviour: Install is presented disabled and maps to install_unavailable, the two honourable actions stay enabled, every command id routes to its declared action, unknown commands return null, ids are distinct, and dismissal defers rather than implying an install. Because windowProc now reads the same declarations the tests assert on, the two cannot drift. QuickChats.zig had zero importers anywhere in the repo. Its Availability enum had a single variant and Controller returned it unconditionally, so "quick chat operations are available through the daemon controller" could not fail without a compile error, while protocolGapMessage emitted "unavailable" strings the Controller could never produce. Its one genuine assertion covered Wire.commandName, so move that into Wire.zig -- the file that owns commandName -- and delete the dead scaffold. The anti-drift guard (issue #424) compared test-bearing sources against a hand-maintained list rather than against the zig test invocations it claimed to describe. Deleting an invocation while leaving its name in the list silently stopped running those tests and still passed: verified by removing the Navigation.zig invocation, which the old guard reported as PASS. Derive the wired set from the script's own invocations so the guard observes reality instead of a description of it, and fail loudly if derivation yields nothing rather than vacuously passing. Evidence (pinned Zig 0.15.2): UpdateOfferDialog 4/4, Wire 61/61, App 233/233. Six mutations of the new dialog assertions all caught; mutating Wire.commandName caught by the relocated assertions; removing an invocation, typo'ing one, dropping a file from a multi-file invocation, and breaking derivation are all now caught, where the first previously passed. Signed-off-by: Colin Neilens <coneilen@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.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.
Summary
Three tests were executing in CI while being structurally incapable of failing, and the guard meant to prevent exactly that could not see the regression it was built to catch. These were surfaced during the parity fleet and deferred as "flag, don't fix" while the relevant PRs were in flight; those have now merged, so this closes them out.
1.
UpdateOfferDialog.zigasserted nothingThe single test read
expectEqual(Action.later, .later)-- an enum compared to itself, invoking zero production code, under a name claiming to cover "install unavailable". The real contracts (Install presented but disabled, command routing) lived insidewindowProc/createButton, unreachable from a unit test.Extracted
buttonsandactionForCommand, and madewindowProcconsume them rather than open-coding the same decisions. The tests assert the same declarations the product reads, so they cannot drift. Incidentally removes the magic couplingif (id == later_id) 110 else 140.2.
QuickChats.zigwas dead code with an unfailable testZero importers repo-wide.
Availabilityhad one variant andControllerreturned it unconditionally, so "quick chat operations are available through the daemon controller" could not fail without a compile error. MeanwhileprotocolGapMessageemitted "unavailable" strings thatControllercould never produce.Its one genuine assertion covered
Wire.commandName, so that moved intoWire.zig-- the file owning the function -- and the dead scaffold was deleted.3. The anti-drift guard could not detect the regression it exists to prevent
The guard (issue #424) compared test-bearing sources against a hand-maintained list, not against the
zig testinvocations that list claimed to describe. Deleting an invocation while leaving its name in the list silently stopped running those tests and still passed the guard.Verified: removing the
Navigation.ziginvocation, old guard reportedPASS. The wired set is now derived from the script's own invocations, and derivation yielding zero results is a hard failure rather than a vacuous pass.Evidence
All under pinned Zig 0.15.2 (local 0.16.0 is actively misleading here).
RED: vacuous assertions could not fail -> 6 mutations of UpdateOfferDialog (Install silently enabled, Install claims real install, routing collapsed to later, unknown command resolves, dismiss implies install, duplicated command id) all passed against the old test and are all caught by the new ones
GREEN: UpdateOfferDialog 4/4, Wire 61/61, App 233/233 -> full consumer compiles and passes after the refactor
REGRESSION: removing the Navigation.zig invocation passed the old guard -> now fails; typo'd invocation, dropping a file from a multi-file invocation, and broken derivation are all caught too
Mutating
Wire.commandNameis caught by the relocated assertions, confirming the moved coverage is real rather than relocated text.Scope
Wired suite count goes 40 -> 39 by deleting one dead file; no file that was executed before stops being executed. (Note: an earlier count of "39 wired suites" circulating from the #426 work was an undercount -- it used a
[A-Za-z]+file-name pattern that silently skippedWin32.zig. The guard now prints its own derived count, so this number is observed rather than asserted: CI reportsPASS (39 source files executed).) Untouched:Sidebar.zig,GraphCanvas.zig,Accessibility.zig,AccessibilityProvider.cpp,uia-live-gate.ps1.Wire.commandNamehas no product callers at all -- its only consumers were these tests. Left in place deliberately rather than cascading into an unrelated deletion.