Skip to content

test(workflow-core): cover isFileResolved and parse owner - #6699

Merged
aglinxinyuan merged 1 commit into
apache:mainfrom
Ma77Ball:test/file-resolver-helpers-spec
Jul 22, 2026
Merged

test(workflow-core): cover isFileResolved and parse owner#6699
aglinxinyuan merged 1 commit into
apache:mainfrom
Ma77Ball:test/file-resolver-helpers-spec

Conversation

@Ma77Ball

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

  • Add unit tests for FileResolver.isFileResolved covering non-empty scheme, missing scheme, and malformed URI cases.
  • Add unit tests for FileResolver.parseDatasetOwnerAndName covering valid paths, too few segments, and a null path.

Any related issues, documentation, discussions?

Closes: #6698

How was this PR tested?

  • Run: sbt "WorkflowCore/testOnly *FileResolverSpec", expect all 9 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:

  • Contributors with relevant context: @Yicong-Huang
    You can notify them by mentioning @Yicong-Huang in a comment.

@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.65%. Comparing base (0467c76) to head (2c9ebde).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6699   +/-   ##
=========================================
  Coverage     74.65%   74.65%           
+ Complexity     3438     3436    -2     
=========================================
  Files          1160     1160           
  Lines         45793    45793           
  Branches       5069     5069           
=========================================
  Hits          34189    34189           
+ Misses         9953     9951    -2     
- Partials       1651     1653    +2     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 76.76% <ø> (ø) Carriedforward from 0467c76
amber 66.81% <ø> (ø)
computing-unit-managing-service 18.00% <ø> (ø)
config-service 66.66% <ø> (ø)
file-service 66.80% <ø> (ø)
frontend 78.56% <ø> (ø) Carriedforward from 0467c76
notebook-migration-service 78.94% <ø> (ø)
pyamber 91.99% <ø> (ø) Carriedforward from 0467c76
workflow-compiling-service 55.14% <ø> (ø)

*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

🟢 0 better · 🔴 9 worse · ⚪ 6 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 381 0.233 25,475/33,639/33,639 us 🔴 -17.4% / 🔴 +115.2%
🔴 bs=100 sw=10 sl=64 942 0.575 105,755/125,540/125,540 us 🔴 +8.9% / 🔴 +18.4%
🔴 bs=1000 sw=10 sl=64 1,093 0.667 904,121/1,030,623/1,030,623 us 🔴 +5.9% / 🟢 -7.2%
Baseline details

Latest main 0467c76 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 381 tuples/sec 461 tuples/sec 767.23 tuples/sec -17.4% -50.3%
bs=10 sw=10 sl=64 MB/s 0.233 MB/s 0.282 MB/s 0.468 MB/s -17.4% -50.2%
bs=10 sw=10 sl=64 p50 25,475 us 21,708 us 12,705 us +17.4% +100.5%
bs=10 sw=10 sl=64 p95 33,639 us 31,885 us 15,633 us +5.5% +115.2%
bs=10 sw=10 sl=64 p99 33,639 us 31,885 us 20,214 us +5.5% +66.4%
bs=100 sw=10 sl=64 throughput 942 tuples/sec 967 tuples/sec 1,001 tuples/sec -2.6% -5.9%
bs=100 sw=10 sl=64 MB/s 0.575 MB/s 0.59 MB/s 0.611 MB/s -2.5% -5.9%
bs=100 sw=10 sl=64 p50 105,755 us 104,301 us 99,949 us +1.4% +5.8%
bs=100 sw=10 sl=64 p95 125,540 us 115,245 us 105,996 us +8.9% +18.4%
bs=100 sw=10 sl=64 p99 125,540 us 115,245 us 117,999 us +8.9% +6.4%
bs=1000 sw=10 sl=64 throughput 1,093 tuples/sec 1,055 tuples/sec 1,033 tuples/sec +3.6% +5.8%
bs=1000 sw=10 sl=64 MB/s 0.667 MB/s 0.644 MB/s 0.631 MB/s +3.6% +5.8%
bs=1000 sw=10 sl=64 p50 904,121 us 950,901 us 974,571 us -4.9% -7.2%
bs=1000 sw=10 sl=64 p95 1,030,623 us 973,223 us 1,016,467 us +5.9% +1.4%
bs=1000 sw=10 sl=64 p99 1,030,623 us 973,223 us 1,048,798 us +5.9% -1.7%
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,524.36,200,128000,381,0.233,25475.17,33639.16,33639.16
1,100,10,64,20,2122.58,2000,1280000,942,0.575,105754.99,125539.68,125539.68
2,1000,10,64,20,18298.07,20000,12800000,1093,0.667,904121.24,1030622.51,1030622.51

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 adds Scala unit test coverage in workflow-core for FileResolver.isFileResolved and FileResolver.parseDatasetOwnerAndName, improving branch coverage for URI-scheme detection and dataset-path parsing logic used by the workflow storage layer.

Changes:

  • Add tests for isFileResolved covering non-empty scheme, missing scheme, and malformed URI inputs.
  • Add tests for parseDatasetOwnerAndName covering valid dataset paths, insufficient segments, and null input.

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

@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
Merged via the queue into apache:main with commit 69286ac Jul 22, 2026
32 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 isFileResolved and parseDatasetOwnerAndName

4 participants