Skip to content

test(workflow-core): extend util and tuple unit test coverage - #6987

Merged
aglinxinyuan merged 1 commit into
apache:mainfrom
aglinxinyuan:test-wfcore-util-cov
Jul 29, 2026
Merged

test(workflow-core): extend util and tuple unit test coverage#6987
aglinxinyuan merged 1 commit into
apache:mainfrom
aglinxinyuan:test-wfcore-util-cov

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds 53 tests across four common/workflow-core classes (88 -> 141 in these suites). No source changes.

  • StackTraceUtils (was 0% covered, new spec): pins the exact rendered output (header + Throwable.toString + frames), the Caused by recursion including a 20-deep chain, per-cause frames, and the null-message case.
  • AttributeTypeUtils: the unsupported-type case _ branches of parseInteger/parseLong/parseDouble/parseBoolean (asserting both the message and the wrapped IllegalArgumentException cause), the previously-unexercised Long/Integer/Timestamp/Boolean input arms, forced-vs-strict divergence (forced "2147483648" wraps to Int.MinValue; "1.9" truncates where strict throws), the trim.toInt == 1 fallback, schema-inference edges (empty, one-way widening, all-null), and tupleCasting null handling.
  • S3StorageClient: uploadPartWithRequest's two payload branches (known contentLength -> fromInputStream, unknown -> readAllBytes) with a two-part ordered round trip, the empty-prefix require guard, and the abort-on-mid-stream-failure path — asserting the exception propagates, no pending upload id remains (proving abortMultipartUpload ran), and no object was published. Follows the suite's existing MinIO-testcontainer approach (no client double exists); no live AWS.
  • LargeBinaryManager: baseUriForExecution formatting, S3-verified re-seeding across executions, clear-on-empty/null making create() fail loudly, and thread-local isolation.

Two branches were deliberately left uncovered rather than forced, and are called out in the code review notes: the DEFAULT_BUCKET.isEmpty arm (a val captured at object init — unreachable without mutating production config) and a real per-key DeleteObjects failure (needs S3 object-lock setup; throwOnDeleteErrors is already tested directly).

Any related issues, documentation, discussions?

Closes #6985.

How was this PR tested?

sbt -java-home <jbr-17> "WorkflowCore/testOnly *S3StorageClientSpec *AttributeTypeUtilsSpec *StackTraceUtilsSpec *LargeBinaryManagerSpec" -> 141 succeeded, 0 failed (4 suites). Test/scalafmtCheck + Test/scalafix --check clean.

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

Generated-by: Claude Code (Opus 4.8 [1M context])

Add 53 tests across four classes (no source changes):
- StackTraceUtils (was 0% covered, new spec): pin the exact rendered output,
  the Caused-by recursion incl. a deep chain, and the null-message case.
- AttributeTypeUtils: the unsupported-type branches of parseInteger/Long/Double/
  Boolean (asserting the wrapped cause), the untested Long/Integer/Timestamp/
  Boolean input arms, forced-vs-strict divergence (overflow wrap, decimal
  truncation), the trim.toInt fallback, schema inference edges, and casting nulls.
- S3StorageClient: uploadPartWithRequest's known-length and unknown-length
  payload branches, the empty-prefix guard, and the abort-on-mid-stream-failure
  path (asserting no pending upload id and no published object).
- LargeBinaryManager: base-URI scoping, re-seeding across executions, clear-on-
  empty/null, and thread-local isolation.
@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: @Yicong-Huang, @kunwp1
    You can notify them by mentioning @Yicong-Huang, @kunwp1 in a comment.

@codecov-commenter

codecov-commenter commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.03%. Comparing base (469e8f0) to head (4fd042a).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6987      +/-   ##
============================================
+ Coverage     78.99%   79.03%   +0.03%     
+ Complexity     3788     3784       -4     
============================================
  Files          1160     1160              
  Lines         46105    46105              
  Branches       5115     5115              
============================================
+ Hits          36419    36437      +18     
+ Misses         8067     8045      -22     
- Partials       1619     1623       +4     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 76.76% <ø> (ø) Carriedforward from 469e8f0
amber 72.31% <ø> (+0.10%) ⬆️
computing-unit-managing-service 20.49% <ø> (ø)
config-service 66.66% <ø> (ø)
file-service 67.21% <ø> (ø)
frontend 82.99% <ø> (ø) Carriedforward from 469e8f0
notebook-migration-service 78.94% <ø> (ø)
pyamber 95.38% <ø> (ø) Carriedforward from 469e8f0
workflow-compiling-service 26.31% <ø> (ø)

*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

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 469e8f0 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 407 0.248 21,046/42,119/42,119 us 🔴 +9.2% / 🔴 +166.5%
🟢 bs=100 sw=10 sl=64 935 0.57 104,291/133,661/133,661 us 🟢 -19.4% / 🔴 +24.5%
bs=1000 sw=10 sl=64 1,089 0.665 910,192/1,008,556/1,008,556 us ⚪ within ±5% / 🟢 -7.2%
Baseline details

Latest main 469e8f0 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 407 tuples/sec 427 tuples/sec 787.55 tuples/sec -4.7% -48.3%
bs=10 sw=10 sl=64 MB/s 0.248 MB/s 0.26 MB/s 0.481 MB/s -4.6% -48.4%
bs=10 sw=10 sl=64 p50 21,046 us 19,277 us 12,255 us +9.2% +71.7%
bs=10 sw=10 sl=64 p95 42,119 us 40,453 us 15,802 us +4.1% +166.5%
bs=10 sw=10 sl=64 p99 42,119 us 40,453 us 19,008 us +4.1% +121.6%
bs=100 sw=10 sl=64 throughput 935 tuples/sec 905 tuples/sec 997.81 tuples/sec +3.3% -6.3%
bs=100 sw=10 sl=64 MB/s 0.57 MB/s 0.552 MB/s 0.609 MB/s +3.3% -6.4%
bs=100 sw=10 sl=64 p50 104,291 us 103,347 us 100,690 us +0.9% +3.6%
bs=100 sw=10 sl=64 p95 133,661 us 165,808 us 107,316 us -19.4% +24.5%
bs=100 sw=10 sl=64 p99 133,661 us 165,808 us 113,823 us -19.4% +17.4%
bs=1000 sw=10 sl=64 throughput 1,089 tuples/sec 1,082 tuples/sec 1,030 tuples/sec +0.6% +5.7%
bs=1000 sw=10 sl=64 MB/s 0.665 MB/s 0.66 MB/s 0.629 MB/s +0.8% +5.7%
bs=1000 sw=10 sl=64 p50 910,192 us 918,164 us 981,213 us -0.9% -7.2%
bs=1000 sw=10 sl=64 p95 1,008,556 us 1,001,809 us 1,027,605 us +0.7% -1.9%
bs=1000 sw=10 sl=64 p99 1,008,556 us 1,001,809 us 1,055,466 us +0.7% -4.4%
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,491.81,200,128000,407,0.248,21046.43,42118.72,42118.72
1,100,10,64,20,2139.88,2000,1280000,935,0.570,104291.05,133660.62,133660.62
2,1000,10,64,20,18364.67,20000,12800000,1089,0.665,910192.19,1008556.49,1008556.49

@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 Jul 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 29, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 29, 2026
Merged via the queue into apache:main with commit b291f75 Jul 29, 2026
34 checks passed
@aglinxinyuan
aglinxinyuan deleted the test-wfcore-util-cov branch July 29, 2026 06:09
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.

Extend workflow-core util and tuple unit test coverage

3 participants