Skip to content

test(frontend): render the dataset file preview's viewer branches - #7406

Merged
aglinxinyuan merged 1 commit into
apache:mainfrom
aglinxinyuan:test-file-renderer-template
Aug 8, 2026
Merged

test(frontend): render the dataset file preview's viewer branches#7406
aglinxinyuan merged 1 commit into
apache:mainfrom
aglinxinyuan:test-file-renderer-template

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

The file renderer's class was already at 91% locally, but its template was at 25% of statements. The template is a viewer switch — each displayX flag selects exactly one preview — and only the CSV path had ever been rendered.

Adds 16 tests that drive each viewer and assert what actually reaches the screen: the four status alerts (each excluding the others), the shared table for CSV and spreadsheets, image plus the click that opens the full-size modal, video, audio, markdown, JSON, plain text, the empty initial state, and the maximized height.

It also pins the && safeFileURL guards on the media branches. Those are load-bearing: the flag is set as soon as the MIME type is known while the object URL is built asynchronously, so rendering on the flag alone emits a source-less <img>/<video>/<audio>.

Verified by mutation, all reverted (template diff empty):

Mutation Result
drop the && safeFileURL guard on the video branch red
drop the same guard on the image branch red
key the markdown branch off displayJson red
remove the image's click handler red
drop displayXlsx from the table guard red
reword the too-large message red
use 100% height when not maximized red

Local coverage for the component directory: 68.34% → 93.57% of statements.

Three details are commented in the spec, each of which cost a debugging round:

  • The first detectChanges() runs ngOnInit, which inspects the empty filePath and settles on "preview unsupported"; flags set beforehand are silently overwritten. The helper clears state via the component's own turnOffAllDisplay() afterwards.
  • Binding [src] makes Angular call DomSanitizer.sanitize, which the existing stub does not provide, so the new block supplies its own.
  • <markdown> needs MarkdownModule.forRoot(), following agent-chat.component.spec.ts.

No production file is touched.

Any related issues, documentation, discussions?

Closes #7403

How was this PR tested?

npx ng test --watch=false --include="**/user-dataset-file-renderer.component.spec.ts"
 Test Files  1 passed (1)
      Tests  48 passed (48)

16 new on top of the existing 32. yarn format:ci passes.

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

Generated-by: Claude Code (Opus 5)

The file renderer's class was already well covered, but its template was not:
the template is a viewer switch where each displayX flag selects exactly one
preview, and only the CSV path had ever been rendered.

Adds 16 tests that drive each viewer and assert what reaches the screen: the
four status alerts, the shared table for CSV and spreadsheets, image (including
the click that opens the full-size modal), video, audio, markdown, JSON, plain
text, the empty initial state, and the maximized height.

Also pins the safeFileURL guards. displayMP4/MP3/Image are set as soon as the
MIME type is known, while the object URL is built asynchronously, so rendering
on the flag alone would emit a source-less media element.

Local coverage for the component directory moves from 68.34% to 93.57% of
statements.

No production file is touched.
Copilot AI lite review requested due to automatic review settings August 8, 2026 01:06
@github-actions github-actions Bot added the frontend Changes related to the frontend GUI label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • No candidates found from git blame history.

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

This PR increases frontend test coverage for UserDatasetFileRendererComponent by adding a dedicated rendering-focused test suite that exercises all template viewer branches (status alerts, tabular previews, media previews with modal interaction, and text viewers), without changing production code.

Changes:

  • Adds a new “rendering” test suite that asserts which viewer elements actually render for each displayX/status flag combination.
  • Introduces a TestBed configuration for rendering tests that includes MarkdownModule.forRoot() and a DomSanitizer stub implementing sanitize to support [src] bindings.
  • Adds assertions for media guards (displayX && safeFileURL), the image modal click interaction, empty initial state, and maximized vs non-maximized height styling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.78%. Comparing base (389db60) to head (f545a57).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7406      +/-   ##
============================================
+ Coverage     84.70%   84.78%   +0.07%     
  Complexity     4151     4151              
============================================
  Files          1169     1169              
  Lines         46740    46740              
  Branches       5202     5202              
============================================
+ Hits          39591    39627      +36     
+ Misses         5433     5401      -32     
+ Partials       1716     1712       -4     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 389db60
agent-service 85.50% <ø> (ø) Carriedforward from 389db60
amber 80.85% <ø> (ø) Carriedforward from 389db60
computing-unit-managing-service 50.72% <ø> (ø) Carriedforward from 389db60
config-service 65.97% <ø> (ø) Carriedforward from 389db60
file-service 69.05% <ø> (ø) Carriedforward from 389db60
frontend 86.63% <ø> (+0.17%) ⬆️
notebook-migration-service 78.89% <ø> (ø) Carriedforward from 389db60
pyamber 97.55% <ø> (ø) Carriedforward from 389db60
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from 389db60

*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.

@aglinxinyuan
aglinxinyuan requested a review from mengw15 August 8, 2026 01:17

@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

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 8, 2026
Merged via the queue into apache:main with commit dce17e8 Aug 8, 2026
25 checks passed
@aglinxinyuan
aglinxinyuan deleted the test-file-renderer-template branch August 8, 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.

Render the dataset file preview's viewer branches

4 participants