test(workflow-core): add unit test coverage for storage-model classes#6095
Merged
Merged
Conversation
Targets Codecov-missed lines (all 0%): ResultSchema (schema column names/types), ReadonlyLocalFileDocument (URI/file/stream accessors + unsupported-op throws, via temp file), and VirtualDocument (every default method throws NotImplementedError with its exact message).
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds ScalaTest unit coverage in common/workflow-core for previously-uncovered storage-model/result-schema classes, without changing production code. The new specs pin schema column order/types and document behavior (including default/unsupported method error messages).
Changes:
- Add unit tests for
ResultSchemaruntime-statistics and console-messages schemas. - Add unit tests for
ReadonlyLocalFileDocumentURI/file accessors, input-stream reads, and unsupported iterator APIs. - Add unit tests for
VirtualDocumentdefault method behavior (expectedNotImplementedErrormessages) via a minimal concrete subclass.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/result/ResultSchemaSpec.scala | Pins ResultSchema attribute names (order) and attribute types for the two exported schemas. |
| common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/VirtualDocumentSpec.scala | Verifies default VirtualDocument methods throw NotImplementedError with exact messages using a minimal concrete implementation. |
| common/workflow-core/src/test/scala/org/apache/texera/amber/core/storage/model/ReadonlyLocalFileDocumentSpec.scala | Verifies read-only local file document accessors, stream reading, and unsupported collection accessors’ exact error messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6095 +/- ##
============================================
+ Coverage 58.17% 58.60% +0.42%
- Complexity 3144 3204 +60
============================================
Files 1132 1132
Lines 43674 43674
Branches 4734 4734
============================================
+ Hits 25407 25594 +187
+ Misses 16831 16657 -174
+ Partials 1436 1423 -13
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 377 | 0.23 | 26,409/37,178/37,178 us | 🔴 +21.3% / 🔴 +142.5% |
| 🔴 | bs=100 sw=10 sl=64 | 915 | 0.558 | 107,275/145,437/145,437 us | 🔴 -5.3% / 🔴 +34.2% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,081 | 0.66 | 920,632/1,008,083/1,008,083 us | ⚪ within ±5% / 🟢 -7.3% |
Baseline details
Latest main a3f22bb from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 377 tuples/sec | 424 tuples/sec | 772.08 tuples/sec | -11.1% | -51.2% |
| bs=10 sw=10 sl=64 | MB/s | 0.23 MB/s | 0.259 MB/s | 0.471 MB/s | -11.2% | -51.2% |
| bs=10 sw=10 sl=64 | p50 | 26,409 us | 21,763 us | 12,745 us | +21.3% | +107.2% |
| bs=10 sw=10 sl=64 | p95 | 37,178 us | 34,082 us | 15,330 us | +9.1% | +142.5% |
| bs=10 sw=10 sl=64 | p99 | 37,178 us | 34,082 us | 19,054 us | +9.1% | +95.1% |
| bs=100 sw=10 sl=64 | throughput | 915 tuples/sec | 964 tuples/sec | 982.64 tuples/sec | -5.1% | -6.9% |
| bs=100 sw=10 sl=64 | MB/s | 0.558 MB/s | 0.589 MB/s | 0.6 MB/s | -5.3% | -7.0% |
| bs=100 sw=10 sl=64 | p50 | 107,275 us | 102,132 us | 101,961 us | +5.0% | +5.2% |
| bs=100 sw=10 sl=64 | p95 | 145,437 us | 139,853 us | 108,335 us | +4.0% | +34.2% |
| bs=100 sw=10 sl=64 | p99 | 145,437 us | 139,853 us | 114,379 us | +4.0% | +27.2% |
| bs=1000 sw=10 sl=64 | throughput | 1,081 tuples/sec | 1,097 tuples/sec | 1,013 tuples/sec | -1.5% | +6.7% |
| bs=1000 sw=10 sl=64 | MB/s | 0.66 MB/s | 0.67 MB/s | 0.618 MB/s | -1.5% | +6.7% |
| bs=1000 sw=10 sl=64 | p50 | 920,632 us | 918,324 us | 993,573 us | +0.3% | -7.3% |
| bs=1000 sw=10 sl=64 | p95 | 1,008,083 us | 961,176 us | 1,032,489 us | +4.9% | -2.4% |
| bs=1000 sw=10 sl=64 | p99 | 1,008,083 us | 961,176 us | 1,065,526 us | +4.9% | -5.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,529.90,200,128000,377,0.230,26408.79,37178.45,37178.45
1,100,10,64,20,2186.36,2000,1280000,915,0.558,107274.52,145437.36,145437.36
2,1000,10,64,20,18499.91,20000,12800000,1081,0.660,920631.69,1008083.20,1008083.20- ReadonlyLocalFileDocument: use the StandardCharsets.UTF_8 Charset overload instead of a stringly-typed charset name - VirtualDocument: assert URI and clear() on a single shared instance
Yicong-Huang
approved these changes
Jul 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Add unit test coverage for three
common/workflow-corestorage-model classes, selected from the Codecov report (all 0%). No production-code changes.ResultSchema.scalaReadonlyLocalFileDocument.scalaVirtualDocument.scalaNotImplementedErrorwith its exact message, via a minimal concrete subclassAny related issues, documentation, discussions?
Follow-up to the review feedback on #6043: prioritize tests that fill uncovered code paths.
How was this PR tested?
sbt "WorkflowCore/testOnly *ResultSchemaSpec *ReadonlyLocalFileDocumentSpec *VirtualDocumentSpec"— 20 tests, all greensbt "WorkflowCore/Test/scalafmtCheck"andsbt "WorkflowCore/scalafixAll --check"— cleanWas this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])