Skip to content

test(workflow-execution-service): cover ResultExportService helpers#6705

Merged
aglinxinyuan merged 2 commits into
apache:mainfrom
Ma77Ball:test/result-export-service-spec
Jul 22, 2026
Merged

test(workflow-execution-service): cover ResultExportService helpers#6705
aglinxinyuan merged 2 commits into
apache:mainfrom
Ma77Ball:test/result-export-service-spec

Conversation

@Ma77Ball

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

  • Add ResultExportServiceSpec covering parseOperators deserialization, empty-array round-trip, and malformed-input failure.
  • Cover validateExportRequest returning a 400 response when no operators are selected and None otherwise.

Any related issues, documentation, discussions?

Closes: #6704

How was this PR tested?

  • Run: sbt "WorkflowExecutionService/testOnly *ResultExportServiceSpec", expect all 5 tests passing.
  • Test-only change; no production code is modified.

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

Co-authored with Claude Opus 4.8 in compliance with ASF

@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:

  • No candidates found from git blame history.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 0467c76 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 396 0.241 24,479/36,080/36,080 us 🔴 +7.3% / 🔴 +128.2%
bs=100 sw=10 sl=64 805 0.491 123,255/150,297/150,297 us ⚪ within ±5% / 🔴 +43.1%
bs=1000 sw=10 sl=64 926 0.565 1,080,412/1,105,202/1,105,202 us ⚪ within ±5% / 🔴 +12.1%
Baseline details

Latest main 0467c76 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 396 tuples/sec 409 tuples/sec 769.76 tuples/sec -3.2% -48.6%
bs=10 sw=10 sl=64 MB/s 0.241 MB/s 0.25 MB/s 0.47 MB/s -3.6% -48.7%
bs=10 sw=10 sl=64 p50 24,479 us 24,682 us 12,663 us -0.8% +93.3%
bs=10 sw=10 sl=64 p95 36,080 us 33,611 us 15,813 us +7.3% +128.2%
bs=10 sw=10 sl=64 p99 36,080 us 33,611 us 19,001 us +7.3% +89.9%
bs=100 sw=10 sl=64 throughput 805 tuples/sec 807 tuples/sec 1,009 tuples/sec -0.2% -20.3%
bs=100 sw=10 sl=64 MB/s 0.491 MB/s 0.493 MB/s 0.616 MB/s -0.4% -20.3%
bs=100 sw=10 sl=64 p50 123,255 us 122,187 us 99,444 us +0.9% +23.9%
bs=100 sw=10 sl=64 p95 150,297 us 150,174 us 105,012 us +0.1% +43.1%
bs=100 sw=10 sl=64 p99 150,297 us 150,174 us 116,467 us +0.1% +29.0%
bs=1000 sw=10 sl=64 throughput 926 tuples/sec 906 tuples/sec 1,044 tuples/sec +2.2% -11.3%
bs=1000 sw=10 sl=64 MB/s 0.565 MB/s 0.553 MB/s 0.637 MB/s +2.2% -11.3%
bs=1000 sw=10 sl=64 p50 1,080,412 us 1,096,025 us 964,034 us -1.4% +12.1%
bs=1000 sw=10 sl=64 p95 1,105,202 us 1,143,147 us 1,007,497 us -3.3% +9.7%
bs=1000 sw=10 sl=64 p99 1,105,202 us 1,143,147 us 1,040,936 us -3.3% +6.2%
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,505.69,200,128000,396,0.241,24478.70,36079.82,36079.82
1,100,10,64,20,2483.69,2000,1280000,805,0.491,123255.01,150296.55,150296.55
2,1000,10,64,20,21599.76,20000,12800000,926,0.565,1080412.17,1105201.58,1105201.58

@codecov-commenter

codecov-commenter commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.84%. Comparing base (0467c76) to head (0377966).
⚠️ Report is 31 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6705      +/-   ##
============================================
+ Coverage     74.65%   74.84%   +0.18%     
- Complexity     3438     3450      +12     
============================================
  Files          1160     1160              
  Lines         45793    45793              
  Branches       5069     5069              
============================================
+ Hits          34189    34276      +87     
+ Misses         9953     9863      -90     
- Partials       1651     1654       +3     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 100267c
agent-service 76.76% <ø> (ø) Carriedforward from 100267c
amber 67.32% <ø> (+0.51%) ⬆️
computing-unit-managing-service 18.00% <ø> (ø) Carriedforward from 100267c
config-service 66.66% <ø> (ø) Carriedforward from 100267c
file-service 66.80% <ø> (ø) Carriedforward from 100267c
frontend 78.56% <ø> (ø) Carriedforward from 100267c
notebook-migration-service 78.94% <ø> (ø) Carriedforward from 100267c
pyamber 91.99% <ø> (ø) Carriedforward from 100267c
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 100267c

*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

Adds Scala unit tests for ResultExportService request helper methods in the Amber web service layer, improving coverage for operator JSON parsing and request validation logic without modifying production code.

Changes:

  • Added ResultExportServiceSpec to test parseOperators JSON deserialization (including empty array and malformed input).
  • Added tests for validateExportRequest returning 400 Bad Request when no operators are selected, and None otherwise.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala Outdated
Comment thread amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala Outdated
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aglinxinyuan
aglinxinyuan enabled auto-merge July 22, 2026 08:51

@aglinxinyuan aglinxinyuan 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 Jul 22, 2026
@aglinxinyuan
aglinxinyuan removed this pull request from the merge queue due to a manual request Jul 22, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 22, 2026
@aglinxinyuan
aglinxinyuan removed this pull request from the merge queue due to a manual request Jul 22, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 22, 2026
@aglinxinyuan
aglinxinyuan removed this pull request from the merge queue due to a manual request Jul 22, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 22, 2026
Merged via the queue into apache:main with commit 87ec5f2 Jul 22, 2026
20 checks passed
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.

Add unit test coverage for ResultExportService request helpers

4 participants