test(frontend): expand spec coverage for download / code-editor / annotation-suggestion#5004
Conversation
…otation-suggestion Closes apache#5003. * `download.service.spec.ts` — rewrite the eight `it.skip(...)` blocks with `firstValueFrom` / `await expect(...).rejects.toThrow(...)` against the same mocks; add new cases for `downloadWorkflow`, single-file `downloadOperatorsResult`, the empty-array error path, `getWorkflowResultDownloadability` (HTTP), default-filename fallback, and `isLogin=false` plumbing. The multi-file `downloadOperatorsResult` case stays `it.skip(...)` for now — the source's `import * as JSZip from "jszip"` form is flagged by the build as `Constructing "JSZip" will crash at run-time because it's an import namespace object`, and vitest reproduces that as `__vite_ssr_import_* is not a constructor`. Once the source switches to a default import the skip can come off. * `code-editor.component.spec.ts` — add language-detection cases for the three V2 Python operator types + plain Java + an unknown type, assert `languageTitle` formatting, and exercise `getCoeditorCursorStyles` for valid clientId / colour shapes. The V2 / unknown types aren't in the shared `mockOperatorMetaData`, so this spec subclasses `StubOperatorMetadataService` with the synthesised schemas. * `annotation-suggestion.component.spec.ts` — new file, six tests: creation, default `@Input` values, `onAccept` / `onDecline` event emission (each independently), and that `@Input` updates take. Verified on both: * `main` — 66 files / 333 pass (was 63 / 269 — +64 specs, +3 files) * `chore/monaco-lsp-v10` (the in-flight v10 upgrade, apache#4997) — 65 files / 297 pass; the same new specs run green there too. Confirms the refactor preserves the externally-observable behaviour these specs exercise.
There was a problem hiding this comment.
Pull request overview
This PR expands the frontend unit test suite to cover previously under-tested behavior in DownloadService, CodeEditorComponent, and the AnnotationSuggestionComponent, primarily by rewriting skipped tests into async/observable-friendly Vitest patterns and adding new spec cases.
Changes:
- Rewrote and expanded
DownloadServicespecs to actively test multiple download flows, error paths, and the downloadability endpoint wiring. - Added constructor/language-detection and coeditor-cursor-style coverage for
CodeEditorComponent. - Introduced a new spec file for
AnnotationSuggestionComponentcovering input defaults and output event emission.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/app/dashboard/service/user/download/download.service.spec.ts | Converts skipped Jasmine-style tests to active Vitest-friendly async tests and adds endpoint/download-result coverage. |
| frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.spec.ts | Adds test cases for operator-type language selection, derived languageTitle, and cursor style generation. |
| frontend/src/app/workspace/component/code-editor-dialog/annotation-suggestion.component.spec.ts | Adds initial unit tests for annotation suggestion inputs and accept/decline emitters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5004 +/- ##
============================================
+ Coverage 42.64% 42.78% +0.14%
Complexity 2188 2188
============================================
Files 1045 1045
Lines 39876 39876
Branches 4205 4205
============================================
+ Hits 17004 17061 +57
+ Misses 21811 21757 -54
+ Partials 1061 1058 -3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- annotation-suggestion.component.spec: explicitly import the standalone component into TestBed so the setup is resilient to Angular's defaults flipping back. - code-editor.component.spec: fail fast at module load if the PythonUDF base schema isn't in mockOperatorMetaData, instead of silently falling back to an empty schema cast. - download.service.spec: move httpMock.verify() into afterEach so every spec (current and future) is checked for outstanding HTTP requests, not just the one downloadability test.
Round 2 of Copilot review on PR apache#5004: - Language-detection coverage missed the `RUDFSource` / `RUDF` -> `r` branch in code-editor.component.ts:147. Augmented the stub metadata with two synthetic R schemas, updated the section comment, and parametrized the existing forEach pattern to assert language="r" / languageTitle="R UDF" for both types. - getCoeditorCursorStyles returns bypassSecurityTrustHtml(...) (a SafeHtml consumed via [innerHTML]), not a SafeStyle. Renamed the comment block and the two test descriptions accordingly.
What changes were proposed in this PR?
Three areas were under-tested in the frontend spec suite; this PR addresses all three as one cohesive change:
DownloadService— went from 8 active specs (allit.skip(...)) to 14 active specs + 1 still-skipped (createZip, blocked by an unrelatedimport * as JSZipinterop bug). New coverage:downloadSingleFile(incl. default-filename fallback +isLogin=false),downloadDataset,downloadDatasetVersion,downloadWorkflow(JSON shape),downloadWorkflowsAsZip, single-filedownloadOperatorsResult, theNo files to downloaderror path,getWorkflowResultDownloadabilityHTTP wiring, plus matching error-notification paths.CodeEditorComponent— went from 1 spec (should create) to 9 specs: operator-type → language detection for the three V2 Python operator types and for plain Java / unknown types; thelanguageTitleformula;getCoeditorCursorStylesfor hex and rgba colours.AnnotationSuggestionComponent— new spec file (6 specs): creation, default inputs, accept / decline event emission, independence of the two emitters,@Inputbinding.Why one test stays skipped
The multi-file
downloadOperatorsResultpath goes throughnew JSZip(), where the production code doesimport * as JSZip from "jszip". The CI build flags this asConstructing "JSZip" will crash at run-time because it's an import namespace object, and vitest reproduces it as__vite_ssr_import_* is not a constructor. The spec is left asit.skip(...)with a comment so it gets re-enabled when the source switches to a default import.Any related issues, documentation, discussions?
Closes #5003.
How was this PR tested?
Ran the suite on both branches under active development — confirms the new specs are stable AND the in-flight v10 refactor on #4997 preserves the externally-observable behaviour these specs exercise.
mainchore/monaco-lsp-v10(#4997)The test-count delta between branches is unrelated PR drift (
mainhas picked up extra non-monaco specs that haven't been merged into the v10 branch yet); the spec files this PR adds run green on both.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.7)