Skip to content

test(workflow-operator): close the bar-chart, sentiment and user-agent gaps - #7989

Open
aglinxinyuan wants to merge 2 commits into
apache:mainfrom
aglinxinyuan:cov/operator-small-gaps
Open

test(workflow-operator): close the bar-chart, sentiment and user-agent gaps#7989
aglinxinyuan wants to merge 2 commits into
apache:mainfrom
aglinxinyuan:cov/operator-small-gaps

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Three small workflow-operator gaps, bundled because none is worth a PR alone. 31 tests across the three specs, one of which is new.

Measured with two WorkflowOperator/jacoco runs, one fresh sbt batch JVM each, rm -rf on the jacoco dir between them, and the identical suite-name filter both times. The before run reproduced all published Codecov percentages exactly (88.2 / 90.5 / 88.0 / 85.2 / 83.3), which validates the method.

File Codecov JaCoCo line-hit
HuggingFaceSentimentAnalysisOpDesc.scala 22/25 = 88.0% → 25/25 = 100% 100% → 100%
BarChartOpDesc.scala 30/34 = 88.2% → 32/34 = 94.1% 33/34 → 34/34 = 100%
RandomUserAgent.java 23/27 = 85.2% → 26/27

+8 fully-covered lines and 9 branch arms closed. Worth noting the shape of the sentiment descriptor: its line-hit was already 100%, so its entire gain is partial arms flipping to hits — arms covered went 7 → 12, missed 5 → 0. That is precisely the case Codecov penalises and line-hit hides.

HashJoinProbeOpExec and OPVersion.java were in the original scope and are absent — neither survived assessment, and padding the bundle with them would have added nothing.

Verification

22 mutations. 19 non-equivalent mutants, all 19 killed, each kill re-derived from scratch with the failing test name and assertion line read out of the ScalaTest XML — the sbt log never names them.

The first draft reported no survivors. It shipped with at least seven live semantic survivors, all seven re-derived here against a hash-verified tree.

Three mutants survived and are recorded as equivalent, deliberately run to prove a point: a label-map pair reorder, an empty-cell fill-tuple reorder, and a getOutputSchemas guard-clause reorder. Each was applied specifically to demonstrate that the new assertions check containment and identity rather than incidental ordering — i.e. they are not over-fitted. The guard-clause one is a pure || over side-effect-free predicates, and is distinct from two sibling mutants that swap the variable inside a clause, both of which die.

A mechanism I had wrong, corrected here

Every brief in this campaign has said that FileScanSourceOpExecSpec aborts inside a git worktree because a worktree's .git is a file, throwing RepositoryNotFoundException in beforeAll. That is wrong. Run unfiltered, the suite runs its 7 tests successfully and then aborts at suite level with:

java.nio.file.FileSystemException: ...test_large_binary.txt: The process cannot access the file
because it is being used by another process

— a Windows file-lock in its own cleanup, because a reader is still open. The RepositoryNotFoundException in the same log comes from OPVersion.<clinit>LogicalOp.getOperatorVersion, is caught by OPVersion's own try/catch, and has nothing to do with the abort.

The exclusion is still the right call for measurement, but the corrected mechanism carries a consequence the wrong one hid: on any unfiltered run that file is leaked into the source tree, so it must be deleted before committing or it trips the licence-header check. The earlier claim that "find confirms none exists" was true only because that suite was never allowed to run.

Module-wide branch totals are not quoted here, because IntervalJoinOpExec's covering spec uses an unseeded RNG whose branch count drifts run to run.

Deliberately not included

BarChartOpDesc keeps two partial lines (108 and 114, mb3/cb3 and mb3/cb5), so 32/34 is its ceiling under a test-only change.

Both descriptor specs carry a JSON round-trip test per house convention, and no descriptor subclass is defined in any spec — PythonCodeRawInvalidTextSpec instantiates every subclass by reflection, so a test-only subtype would break it.

The new spec carries the Apache licence header. No production file is touched, and no stray test_large_binary.txt was left behind.

Any related issues, documentation, discussions?

Closes #7987

How was this PR tested?

sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.visualization.barChart.BarChartOpDescSpec org.apache.texera.amber.operator.huggingFace.HuggingFaceSentimentAnalysisOpDescSpec org.apache.texera.amber.operator.source.fetcher.RandomUserAgentSpec"
[info] Total number of tests run: 31
[info] Tests: succeeded 31, failed 0, canceled 0, ignored 0, pending 0

Both full-module runs were green (2365 → 2379 tests, 289 → 290 suites, zero failures), so neither report is the all-zero artifact. Test/scalafmtCheck passes.

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

Generated-by: Claude Code (Opus 5)

…t gaps

Three small files, 31 tests: +8 fully-covered lines and 9 branch arms.
HuggingFaceSentimentAnalysisOpDesc reaches 100% while its line-hit was
already 100% -- the whole gain is partial arms flipping, the case Codecov
penalises and line-hit hides. The before run reproduced all five published
Codecov percentages exactly, which validates the method.

HashJoinProbeOpExec and OPVersion.java were in scope and are absent;
neither survived assessment.

22 mutations, 19 non-equivalent, all 19 killed. The first pass reported no
survivors and shipped with at least seven live ones. Three surviving mutants
are recorded as equivalent and were run deliberately, to show the new
assertions check containment and identity rather than incidental ordering.

Corrects a mechanism this campaign had wrong throughout:
FileScanSourceOpExecSpec does not abort on a worktree's .git file. It runs
its tests and then aborts on a Windows file-lock in its own cleanup, which
means the test_large_binary.txt fixture IS leaked on any unfiltered run and
must be deleted before committing.
Copilot AI lite review requested due to automatic review settings August 26, 2026 06:02

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

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: @kz930
    You can notify them by mentioning @kz930 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 92.92%. Comparing base (5de9bf9) to head (6417ff0).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7989      +/-   ##
============================================
- Coverage     93.21%   92.92%   -0.29%     
- Complexity     4664     4678      +14     
============================================
  Files          1175     1175              
  Lines         47646    47645       -1     
  Branches       5319     5319              
============================================
- Hits          44413    44274     -139     
- Misses         1752     1858     +106     
- Partials       1481     1513      +32     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 98.95% <ø> (-0.38%) ⬇️ Carriedforward from ab1a8ba
amber 89.39% <ø> (+0.07%) ⬆️
computing-unit-managing-service 73.67% <ø> (ø)
config-service 86.73% <ø> (ø)
file-service 86.70% <ø> (ø)
frontend 95.03% <ø> (-0.65%) ⬇️ Carriedforward from ab1a8ba
notebook-migration-service 79.31% <ø> (ø)
pyamber 97.59% <ø> (-0.16%) ⬇️ Carriedforward from ab1a8ba
workflow-compiling-service 77.19% <ø> (ø)

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

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

✅ No material benchmark regressions detected

🟢 0 better · 🔴 0 worse · ⚪ 15 noise (<±5%) · 0 without baseline

Compared against main 5de9bf9 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
bs=10 sw=10 sl=64 612 0.374 15,832/20,968/20,968 us ⚪ within ±5% / 🔴 +54.1%
bs=100 sw=10 sl=64 1,283 0.783 78,090/90,860/90,860 us ⚪ within ±5% / 🟢 -8.4%
bs=1000 sw=10 sl=64 1,465 0.894 677,570/772,429/772,429 us ⚪ within ±5% / 🟢 +19.9%
Baseline details

Latest main 5de9bf9 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 612 tuples/sec 636 tuples/sec 908.02 tuples/sec -3.8% -32.6%
bs=10 sw=10 sl=64 MB/s 0.374 MB/s 0.388 MB/s 0.554 MB/s -3.6% -32.5%
bs=10 sw=10 sl=64 p50 15,832 us 15,918 us 10,907 us -0.5% +45.2%
bs=10 sw=10 sl=64 p95 20,968 us 20,141 us 13,604 us +4.1% +54.1%
bs=10 sw=10 sl=64 p99 20,968 us 20,141 us 16,697 us +4.1% +25.6%
bs=100 sw=10 sl=64 throughput 1,283 tuples/sec 1,272 tuples/sec 1,188 tuples/sec +0.9% +8.0%
bs=100 sw=10 sl=64 MB/s 0.783 MB/s 0.776 MB/s 0.725 MB/s +0.9% +8.0%
bs=100 sw=10 sl=64 p50 78,090 us 78,298 us 85,003 us -0.3% -8.1%
bs=100 sw=10 sl=64 p95 90,860 us 92,040 us 91,599 us -1.3% -0.8%
bs=100 sw=10 sl=64 p99 90,860 us 92,040 us 99,202 us -1.3% -8.4%
bs=1000 sw=10 sl=64 throughput 1,465 tuples/sec 1,480 tuples/sec 1,221 tuples/sec -1.0% +19.9%
bs=1000 sw=10 sl=64 MB/s 0.894 MB/s 0.903 MB/s 0.745 MB/s -1.0% +19.9%
bs=1000 sw=10 sl=64 p50 677,570 us 666,653 us 837,593 us +1.6% -19.1%
bs=1000 sw=10 sl=64 p95 772,429 us 749,788 us 881,015 us +3.0% -12.3%
bs=1000 sw=10 sl=64 p99 772,429 us 749,788 us 915,550 us +3.0% -15.6%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,326.72,200,128000,612,0.374,15832.46,20967.59,20967.59
1,100,10,64,20,1558.48,2000,1280000,1283,0.783,78090.21,90860.39,90860.39
2,1000,10,64,20,13650.80,20000,12800000,1465,0.894,677569.97,772429.44,772429.44

@aglinxinyuan
aglinxinyuan requested a review from mengw15 August 26, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Close the bar-chart, sentiment and user-agent coverage gaps

3 participants