test(frontend): cover the code editor's genuinely untested half - #7735
Open
aglinxinyuan wants to merge 1 commit into
Open
test(frontend): cover the code editor's genuinely untested half#7735aglinxinyuan wants to merge 1 commit into
aglinxinyuan wants to merge 1 commit into
Conversation
Contributor
Automated Reviewer SuggestionsBased on the
|
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 changes were proposed in this PR?
code-editor.component.tsis the largest uncovered file in the repo. Its reported 47.4% is misleading: the component's tests are split across two targets, and only one of them reaches Codecov.angular.jsonhasgui:test(jsdom, whichexcludes**/*.browser.spec.ts) andgui:test-browser(Playwright/Chromium, which includes only those).build.yml:164runs the browser target with no--coverage, and the upload takes only the jsdom lcov. Measuring both and taking the per-line union over the 247 instrumented lines:So 83 lines were already exercised and merely unreported; 50 were genuinely untested. Only the latter is worth writing, and that is what this PR does — plus the 6 genuinely untested lines in the template.
code-editor.component.tscode-editor.component.htmlTests: jsdom 25 -> 65; the browser target 18 -> 22. 46 of the 50 genuinely-untested lines are now covered. The bulk deliberately went into the jsdom spec, since that is the only suite Codecov currently reads — browser-spec work scores zero today.
Covered: the coeditor cursor-style generator and both of its input-sanitising guards, the bulk type-annotation walk and its same-line/new-line offset handling, the suggestion panel's staging and routing, container clamping against the viewport, and the template's
*ngForand accept/decline wiring.A blocker that PR #7586 needs
ng run gui:test-browser --coveragefails out of the box:TypeError: Failed to fetch dynamically imported module: /@id/@vitest/coverage-v8/browser, because Vite does not pre-bundle the coverage provider's browser entry. The one-line fix isoptimizeDeps.include: ["buffer", "@vitest/coverage-v8/browser"]invitest.browser.config.ts. It was applied here only to take the measurement above and then reverted — this PR touches no config. Flagging it because enabling the browser-coverage upload without it would produce an empty report.Verification
12 mutations, 12 killed, no survivors. Each anchor was pre-verified to occur exactly once in a dry run; one mutation at a time;
git diffon production confirmed empty after every revert.replace("0.8", "0.5")SAFE_CLIENT_ID\d{1,10}->\d{1,12}SAFE_CSS_COLORdrop trailing$position.top/position.leftrect.left/rect.topin the clamp*ngForrenders only the first coeditor(accept)/(decline)handlers1..<=n->0..<n===->!==No
vi.mockwas added — the private editor-consuming methods are driven with a plain recording stand-in, given theisolate:falsehazard where the first importer of a module pins it for the whole run.Deliberately not included
10 lines remain uncovered: the
monacoWorkerFactorylabel switch, the dynamic codingame extension imports, the retry-clearing catch, and the LSP timeout reject. All sit behindensureVscodeApiStarted(), a process-wide singleton both suites stub; reaching them means booting the real codingame stack, which the browser spec's own header explicitly declines to do.A production observation, reported and not pinned:
acceptCurrentAnnotation()emits onuserResponseSubjectbefore clearingshowAnnotationSuggestion. The bulk "Add All Type Annotations" walk works only becausegetTypeAnnotationsanswers asynchronously over HTTP — the next suggestion lands after the clear. A first draft of the fixture used a synchronousof(...)and reproduced the failure exactly: the walk stalls after the first argument with the panel hidden. Not a live bug, but if that call ever gains a cache or a local model it breaks silently.rejectCurrentAnnotation()clears first, then emits, so it is not exposed. The fixture models the real async service and carries a comment explaining the dependency.No production file is touched.
Any related issues, documentation, discussions?
Closes #7734
How was this PR tested?
Both suites run twice with identical results. The whole
code-editor-dialogdirectory is green at 113 tests across 4 spec files.yarn format:cipasses.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)