Skip to content

test(frontend): cover the result, export and runtime-statistics services - #7993

Merged
aglinxinyuan merged 2 commits into
apache:mainfrom
aglinxinyuan:cov/result-export-and-runtime-stats
Aug 27, 2026
Merged

test(frontend): cover the result, export and runtime-statistics services#7993
aglinxinyuan merged 2 commits into
apache:mainfrom
aglinxinyuan:cov/result-export-and-runtime-stats

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Four frontend services and one component, bundled because each is only ~3 lines alone. Measured with the CI command itself (nx test --coverage --coverage-reporters=lcovonly), before state restored per-file via git show HEAD:<path>.

+11 fully-covered lines and +10 branch arms. Three of the four files reach 100%; workflow-result.service.ts reaches 99.15%.

Of those +11, nine are reachable from the real UI and two are defensive defaults on public methods. I would rather split them than present all eleven as equivalent. The weaker two: workflow-runtime-statistics.component.ts:137 is an out-of-range tab index that the template cannot produce — it renders exactly 8 tabs for 8 metric keys — but it is a public method reachable with a legal argument, so it counts rather than being refused.

user-dataset-file-renderer.component.ts was in scope and is absent, contributing zero: two dead ??/|| fallbacks already gated by isPreviewSupported, plus an unreachable partial. workflow-action.service.ts was excluded from the outset for the same reason — its three partials compare a Y.Map value against a freshly-constructed {x, y} by reference, so the guard cannot fire in production.

A dead-code defect found while assessing, reported not pinned

The empty-row filter in user-dataset-file-renderer's loadTabularFile is entirely dead. for (const cell in row) enumerates array index strings"0", "1", … — never "", so cell != "" is always true, areCellAllEmpty is always false, and no row is ever filtered. Verified directly in node: a table containing an all-empty row and an empty row keeps both, with the empty one padded out — contradicting the code's own "filter out all empty row" comment.

Fixing it means iterating values with for...of, which is a production change. The current behaviour is deliberately not pinned, so a fix will not have to fight a test.

Verification

22 mutations, 21 killed, 1 equivalent survivor (workflow-result.service.ts:222, an exhaustive-union arm whose branch body is unreachable).

The first draft claimed survivors: [] on ui-udf-parameters-sync.service.ts at "100.00% Codecov". The percentage was metric-true — independently reproduced at 54/54 — but the file carried two real holes behind it. That is the pattern worth naming: a file can be fully executed and still barely constrained.

Two further corrections: the bundle presented all +11 lines as equal in quality (split above), and one uniqueness claim was accurate as far as it went but incomplete about which sibling tests also failed.

Deliberately not included

Three arms in the dropped renderer are unreachable, and any mutation confined to them is equivalent by construction — so none was attempted rather than being reported as a survivor.

Two constraints shaped what was possible here, both worth recording. @angular/build's unit-test runner hard-codes isolate: false, so every spec shares one module registry: that rules out mutating an exported const at test time (it would leak into every other importer), and it is why no vi.mock for Plotly was added to the runtime-statistics spec — per #6580 that pattern is green solo and red on CI. The existing mock-free real-Plotly pattern is used instead.

No production file is touched.

Any related issues, documentation, discussions?

Closes #7991

How was this PR tested?

npx ng test --watch=false --include="**/ui-udf-parameters-sync.service.spec.ts" --include="**/workflow-result.service.spec.ts" --include="**/workflow-result-export.service.spec.ts" --include="**/workflow-runtime-statistics.component.spec.ts"
 Test Files  4 passed (4)

yarn format:ci passes. frontend/junit.xml and frontend/coverage/ are regenerated by every run and are not committed; the jsdom getContext not implemented noise from Monaco is pre-existing.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

+11 fully-covered lines and +10 branch arms across four files; three reach
100%. Of the eleven, nine are reachable from the real UI and two are
defensive defaults on public methods -- split in the PR body rather than
presented as equivalent.

user-dataset-file-renderer was in scope and is absent at zero lines, and
workflow-action.service was excluded from the outset: its three partials
compare a Y.Map value against a freshly-constructed {x, y} by reference, so
the guard cannot fire in production.

Assessing the renderer found dead code, reported not pinned: the empty-row
filter in loadTabularFile can never fire, because `for (const cell in row)`
enumerates array index strings and never the empty string, so no row is ever
filtered despite the comment saying otherwise.

22 mutations, 21 killed, one equivalent. The first pass claimed no survivors
on a file it had taken to 100%; that file carried two real holes behind the
percentage.
Copilot AI lite review requested due to automatic review settings August 26, 2026 08:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @carloea2
    You can notify them by mentioning @carloea2 in a comment.

@codecov-commenter

codecov-commenter commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.27%. Comparing base (5de9bf9) to head (03d005d).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7993      +/-   ##
============================================
+ Coverage     93.21%   93.27%   +0.06%     
  Complexity     4664     4664              
============================================
  Files          1175     1177       +2     
  Lines         47646    47674      +28     
  Branches       5319     5321       +2     
============================================
+ Hits          44413    44470      +57     
+ Misses         1752     1741      -11     
+ Partials       1481     1463      -18     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 456d754
agent-service 99.32% <ø> (ø) Carriedforward from 456d754
amber 89.31% <ø> (ø) Carriedforward from 456d754
computing-unit-managing-service 73.67% <ø> (ø) Carriedforward from 456d754
config-service 86.73% <ø> (ø) Carriedforward from 456d754
file-service 86.70% <ø> (ø) Carriedforward from 456d754
frontend 95.82% <ø> (+0.14%) ⬆️
notebook-migration-service 79.31% <ø> (ø) Carriedforward from 456d754
pyamber 97.74% <ø> (ø) Carriedforward from 456d754
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 456d754

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

@mengw15 mengw15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xinyuan Lin <xinyual3@uci.edu>
@aglinxinyuan
aglinxinyuan enabled auto-merge August 27, 2026 01:15
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 27, 2026
Merged via the queue into apache:main with commit 57a4230 Aug 27, 2026
22 checks passed
@aglinxinyuan
aglinxinyuan deleted the cov/result-export-and-runtime-stats branch August 27, 2026 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cover the result, export and runtime-statistics services

4 participants