test(amber): add unit tests for SleepOpDesc, IntersectOpDesc, URLFetcherOpDesc#4816
Merged
Merged
Conversation
…herOpDesc Closes apache#4814 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
aglinxinyuan
approved these changes
May 3, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4816 +/- ##
=========================================
Coverage 43.95% 43.96%
- Complexity 2131 2138 +7
=========================================
Files 957 957
Lines 34072 34068 -4
Branches 3753 3753
=========================================
+ Hits 14977 14978 +1
+ Misses 18308 18301 -7
- Partials 787 789 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds ScalaTest unit specs for three Amber logical operator descriptors (SleepOpDesc, IntersectOpDesc, URLFetcherOpDesc) to improve WorkflowOperator module coverage and pin current descriptor/physical-op wiring behavior.
Changes:
- Add
SleepOpDescSpecvalidating operator metadata and non-parallelizable single-worker physical-op settings. - Add
IntersectOpDescSpecvalidating metadata, hash partition requirements, and derived output partition behavior. - Add
URLFetcherOpDescSpecvalidating metadata, decoding-dependentsourceSchema, and basic physical-op wiring.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpDescSpec.scala | Adds descriptor-level tests for URL Fetcher operatorInfo, sourceSchema branches, and physical-op wiring. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/sleep/SleepOpDescSpec.scala | Adds descriptor-level tests for Sleep operatorInfo and physical-op flags/class wiring. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/intersect/IntersectOpDescSpec.scala | Adds descriptor-level tests for Intersect operatorInfo, partition requirements, and exec wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n check - Replace `physical.opExecInitInfo.toString should include(...)` in the Sleep, Intersect, and URLFetcher specs with a pattern match on `OpExecWithClassName(className, _)` plus `className shouldBe FQCN`, so the assertions only fail on actual wiring changes (not on scalapb toString formatting). - Guard the URLFetcher default-decodingMethod test with an explicit `schema.getAttributes should have length 1` before `.head` so a future shape change surfaces a meaningful failure instead of a bare `NoSuchElementException`. - Promote the URLFetcher schema-propagation test from a port-count comparison to actually invoking `physical.propagateSchema.func` and asserting the returned mapping contains the output port id with a schema equal to `op.sourceSchema()`.
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?
Adds scalatest coverage for three logical-operator descriptors that had no dedicated specs:
sleep/SleepOpDesc— operatorInfo (Sleep, Control group, 1 input + 1 output) andgetPhysicalOp(non-parallelizable, single-worker, SleepOpExec class wired in).intersect/IntersectOpDesc— operatorInfo (Intersect, Set group, 2 input ports with PortIdentity 0/1, blocking output) andgetPhysicalOp(HashPartition required on both inputs, derived HashPartition for any input combination).source/fetcher/URLFetcherOpDesc— operatorInfo (URL Fetcher, API group, source-shaped 0-in/1-out),sourceSchema(UTF-8 → STRING, RAW_BYTES → ANY, null fallthrough → ANY), andgetPhysicalOp(URLFetcherOpExec class wired in).Any related issues, documentation, discussions?
Closes #4814.
Bug filed separately:
URLFetcherOpDesc.sourceSchemasilently falls back toANYwhendecodingMethodis left at itsnulldefault rather than guarding non-null. Pinned in the spec as the current behavior.Drive-by note (already filed as #4813):
source/scan/json/JSONUtil.scalais a byte-for-byte duplicate ofworkflow-core/util/JSONUtils.scala::JSONToMap. Not adding a redundant spec for it; its tests live withJSONUtilsSpecfrom #4716.How was this PR tested?
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-opus-4-7)