test(amber): extend WorkflowVersionResource unit test coverage - #7166
Merged
Conversation
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands unit test coverage for WorkflowVersionResource (Amber backend) by extending the existing MockTexeraDB-backed WorkflowVersionResourceSpec to cover previously untested resource methods and diff/importance logic. It aligns with the goal in #7161 to add coverage without modifying production code.
Changes:
- Added DB-backed setup for owner/stranger users and workflow access grants to exercise auth-gated endpoints.
- Added unit tests for core versioning helpers (
applyPatch,isSnapshotInRangeUnimportant,insertVersion,insertNewVersion,getLatestVersion). - Added resource endpoint tests for version listing/retrieval and cloning (
retrieveVersionsOfWorkflow,retrieveWorkflowVersion,cloneVersion).
💡 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 #7166 +/- ##
============================================
+ Coverage 79.62% 79.79% +0.16%
- Complexity 3836 3839 +3
============================================
Files 1160 1160
Lines 46188 46188
Branches 5145 5145
============================================
+ Hits 36777 36854 +77
+ Misses 7778 7699 -79
- Partials 1633 1635 +2
*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 | 409 | 0.249 | 23,184/34,781/34,781 us | 🔴 +12.4% / 🔴 +120.3% |
| 🟢 | bs=100 sw=10 sl=64 | 836 | 0.51 | 117,926/144,566/144,566 us | 🟢 -17.2% / 🔴 +31.1% |
| ⚪ | bs=1000 sw=10 sl=64 | 963 | 0.587 | 1,038,559/1,076,698/1,076,698 us | ⚪ within ±5% / ⚪ within ±5% |
Baseline details
Latest main 45b7b20 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 409 tuples/sec | 425 tuples/sec | 771.62 tuples/sec | -3.8% | -47.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.249 MB/s | 0.26 MB/s | 0.471 MB/s | -4.2% | -47.1% |
| bs=10 sw=10 sl=64 | p50 | 23,184 us | 20,626 us | 12,626 us | +12.4% | +83.6% |
| bs=10 sw=10 sl=64 | p95 | 34,781 us | 34,135 us | 15,786 us | +1.9% | +120.3% |
| bs=10 sw=10 sl=64 | p99 | 34,781 us | 34,135 us | 19,344 us | +1.9% | +79.8% |
| bs=100 sw=10 sl=64 | throughput | 836 tuples/sec | 819 tuples/sec | 971.56 tuples/sec | +2.1% | -14.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.51 MB/s | 0.5 MB/s | 0.593 MB/s | +2.0% | -14.0% |
| bs=100 sw=10 sl=64 | p50 | 117,926 us | 116,506 us | 103,463 us | +1.2% | +14.0% |
| bs=100 sw=10 sl=64 | p95 | 144,566 us | 174,526 us | 110,296 us | -17.2% | +31.1% |
| bs=100 sw=10 sl=64 | p99 | 144,566 us | 174,526 us | 118,690 us | -17.2% | +21.8% |
| bs=1000 sw=10 sl=64 | throughput | 963 tuples/sec | 962 tuples/sec | 1,001 tuples/sec | +0.1% | -3.8% |
| bs=1000 sw=10 sl=64 | MB/s | 0.587 MB/s | 0.587 MB/s | 0.611 MB/s | 0.0% | -4.0% |
| bs=1000 sw=10 sl=64 | p50 | 1,038,559 us | 1,037,514 us | 1,008,988 us | +0.1% | +2.9% |
| bs=1000 sw=10 sl=64 | p95 | 1,076,698 us | 1,076,848 us | 1,055,260 us | -0.0% | +2.0% |
| bs=1000 sw=10 sl=64 | p99 | 1,076,698 us | 1,076,848 us | 1,074,689 us | -0.0% | +0.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,489.32,200,128000,409,0.249,23184.43,34781.27,34781.27
1,100,10,64,20,2393.38,2000,1280000,836,0.510,117926.41,144565.84,144565.84
2,1000,10,64,20,20778.34,20000,12800000,963,0.587,1038558.94,1076698.14,1076698.14
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?
Extends the existing
MockTexeraDB-backedWorkflowVersionResourceSpec(whichonly covered
fetchSubsequentVersions+applyPatch) to cover the resource'sremaining methods. 14 new tests:
applyPatchreconstructs an older content from a storeddelta (no DB);
isSnapshotInRangeUnimportantreturns true for equal bounds andfor a replace-only range, false when the range contains an important (non-replace)
change.
insertNewVersionpersists aretrievable version;
getLatestVersionreturns the highest vid, and creates onewhen the workflow has none;
insertVersionalways versions a new workflow, andotherwise only when the content actually changed.
retrieveVersionsOfWorkflowreturns theimportance-encoded list for a user with access and an empty list otherwise;
retrieveWorkflowVersionreconstructs the workflow for an authorized user andthrows
ForbiddenExceptionotherwise;cloneVersioncreates a new workflow froman existing version.
The spec's
beforeEachnow also seeds an owner + a stranger user with theappropriate access grants (mirroring
WorkflowResourceCoverSpec) so theauth-gated endpoints can be exercised. No production code was changed.
Any related issues, documentation, discussions?
Closes #7161.
How was this PR tested?
sbt "WorkflowExecutionService/testOnly *WorkflowVersionResourceSpec"— 15 passed(1 existing + 14 new), run twice for determinism against the embedded Postgres.
The failure path was verified by breaking an assertion (1 failed, non-zero exit),
and
Test/scalafmtCheck+Test/scalafix --checkare clean.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])