test(review): repair guide virtualization mock and run its tests in CI - #1200
Merged
Conversation
The AllFilesCodeView lifecycle test stubs '@pierre/diffs/react' with mock.module, which replaces the whole specifier. #1193 added an EditProvider import to AllFilesCodeView, but the stub was never extended, so the file throws at import when run on its own. The three DOM tests #1158 added for Guided Review virtualization were also never registered in the DOM_TESTS=1 CI step, so nothing enforced the 8-viewer mount cap and nothing surfaced the broken stub.
1 task
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.
What broke
AllFilesCodeView.lifecycle.test.tsxstubs@pierre/diffs/reactwithmock.module, which replaces the entire specifier. #1193 added anEditProviderimport toAllFilesCodeView.tsx, but the stub was not extended, so the file threwSyntaxError: Export named 'EditProvider' not foundat import. The stub now provides every value export the component reads.Why nobody noticed
Two reasons stacked. The file was masked in any run that also loaded
useEditSession.recovery.test.tsxfirst, which pulls in the real module. And the three DOM tests #1158 added for Guided Review virtualization were never registered in theDOM_TESTS=1CI step, so plainbun testskipped them and they never ran at all.What the tests guard
GuideViewportManagercaps live Pierre CodeView instances at 8 while Guided Review renders one card per file. Before #1158 all of them mounted: roughly 1.07M shadow DOM elements, 770MB heap, 3.9 FPS.GuideView.test.tsxandGuideSectionCard.test.tsxassert the cap holds.CI cost
The DOM step goes from about 14.8s to about 16.9s locally, a delta of roughly +2.1s.
Verified non-vacuous: lifting the enforced cap while pinning the assertion bound at 8 fails two
GuideView.test.tsxcases (20 and 9 mounted views). Probe reverted. Full suite: 2875 pass, 0 fail.