aglinxinyuan opened a new pull request, #7735:
URL: https://github.com/apache/texera/pull/7735
### What changes were proposed in this PR?
`code-editor.component.ts` is 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.json` has `gui:test` (jsdom, which `exclude`s
`**/*.browser.spec.ts`) and `gui:test-browser` (Playwright/Chromium, which
includes only those). `build.yml:164` runs 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:
| suite | covered | missed | % |
|---|---|---|---|
| jsdom only (what Codecov sees) | 114 | 133 | 46.2% |
| browser only | 156 | 91 | 63.2% |
| **true union** | **197** | **50** | **79.8%** |
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.
| file | Codecov-visible (jsdom) | true union |
|---|---|---|
| `code-editor.component.ts` | 46.2% -> **77.7%** | 79.8% -> **96.0%** |
| `code-editor.component.html` | 70.0% -> **100%** | 70.0% -> **100%** |
Tests: 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 `*ngFor` and
accept/decline wiring.
### A blocker that PR #7586 needs
`ng run gui:test-browser --coverage` **fails 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 is `optimizeDeps.include: ["buffer",
"@vitest/coverage-v8/browser"]` in `vitest.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 diff` on
production confirmed empty after every revert.
| Mutation | Killed by |
|---|---|
| **exchange** the two operands of `replace("0.8", "0.5")` | scopes every
rule to the coeditor id and dims only the selection background |
| `SAFE_CLIENT_ID` `\d{1,10}` -> `\d{1,12}` | emits no CSS for a clientId
longer than ten digits |
| `SAFE_CSS_COLOR` drop trailing `$` | emits no CSS for a colour carrying a
style-tag escape |
| **exchange** the same-line / new-line offset branch bodies | offsets a
same-line argument and resets on a new line |
| **exchange** `position.top` / `position.left` | stages the trimmed
suggestion into the rendered panel |
| **exchange** code / suggestion pushed to the panel | same test, distinct
assertion |
| **exchange** `rect.left` / `rect.top` in the clamp | clamps a container
overflowing on both axes |
| `*ngFor` renders only the first coeditor | renders one scoped cursor-style
block per coeditor |
| **exchange** the `(accept)` / `(decline)` handlers | routes accept and
decline to the matching handlers |
| tokenize sweep `1..<=n` -> `0..<n` | (browser) force-tokenizes every line |
| diff operator predicate `===` -> `!==` | (browser) diffs this operator's
latest version against the shared text |
| **exchange** the diff / plain bring-up branches | 10 tests |
No `vi.mock` was added — the private editor-consuming methods are driven
with a plain recording stand-in, given the `isolate:false` hazard where the
first importer of a module pins it for the whole run.
### Deliberately not included
10 lines remain uncovered: the `monacoWorkerFactory` label switch, the
dynamic codingame extension imports, the retry-clearing catch, and the LSP
timeout reject. All sit behind `ensureVscodeApiStarted()`, 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 on `userResponseSubject` **before** clearing
`showAnnotationSuggestion`. The bulk "Add All Type Annotations" walk works only
because `getTypeAnnotations` answers asynchronously over HTTP — the next
suggestion lands after the clear. A first draft of the fixture used a
synchronous `of(...)` 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?
```
npx ng test --watch=false --include="**/code-editor.component.spec.ts"
npx ng run gui:test-browser
```
```
Test Files 1 passed (1)
Tests 65 passed (65)
Test Files 3 passed (3)
Tests 22 passed (22)
```
Both suites run twice with identical results. The whole `code-editor-dialog`
directory is green at 113 tests across 4 spec files. `yarn format:ci` passes.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
