mengw15 opened a new pull request, #8288:
URL: https://github.com/apache/texera/pull/8288

   ### What changes were proposed in this PR?
   
   `workflow-execution-history.component.spec.ts` let `ngAfterViewInit` call 
the real `Plotly.newPlot` on every setup. 36 of its 61 tests build the 
component, and each build plots twice, so one run drove roughly 72 real Plotly 
renders through jsdom — for the benefit of a single test.
   
   That test then read `data` and `layout` back off the graph divs Plotly 
populates. Every value it asserted (`type`, `labels`, `values`, `x`, `y`, the 
widths, the titles) is one the component itself computed and handed over, so 
the round trip through the real library bought no coverage. It also cannot buy 
any here: jsdom has no layout or canvas, so whether Plotly drew anything 
sensible is unobservable — that belongs to the browser-mode suite, if anywhere.
   
   Mock the module for this spec and assert on the arguments passed to 
`newPlot` instead. The chart test now checks the component's own decision 
rather than Plotly's handling of it.
   
   This is what made the spec flaky rather than merely slow: the failing case 
on macOS was three synchronous assertions behind a `setup()`, timing out 
against the 20 s per-test limit under runner contention, while ubuntu and 
windows passed. Removing the dominant avoidable cost restores the margin; it is 
not a proof that Plotly was the only contributor.
   
   `user-quota` and `workflow-runtime-statistics` render real Plotly too, but 
at 339 ms / 25 tests and 197 ms / 12 tests they are nowhere near the limit, so 
they are left alone.
   
   Also corrects `TESTING.md`, which documented `ng test --test-file <path>` 
for single-file runs; the builder rejects that argument and the flag is 
`--include`.
   
   ### Any related issues, documentation, discussions?
   
   Closes #8287.
   
   ### How was this PR tested?
   
   `yarn ng test --include ".../workflow-execution-history.component.spec.ts"` 
— 61 passed, test time 4.12 s → 1.69 s.
   
   Failure paths checked, since the rewritten assertions must still catch what 
the old ones did: changing the component's pie-chart `WIDTH` from 450 to 451 
turns the chart test red, and removing one of the two `Plotly.newPlot` calls 
does too. Restoring each returns the file to 61 passing. `eslint` is clean on 
the touched spec.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (claude-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]

Reply via email to