test(workflow-operator): add unit test coverage for visualization option enums#6043
Closed
aglinxinyuan wants to merge 1 commit into
Closed
test(workflow-operator): add unit test coverage for visualization option enums#6043aglinxinyuan wants to merge 1 commit into
aglinxinyuan wants to merge 1 commit into
Conversation
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the common/workflow-operator test suite by adding unit tests that “pin” the JSON wire-format and Jackson round-trip behavior for several visualization option enums used by operators and UI/property-window configuration.
Changes:
- Added ScalaTest specs to verify each enum constant’s
@JsonValuemapping and expected constant count. - Added Jackson serialization/deserialization round-trip tests for each enum using the shared
JSONUtils.objectMapper. - Added targeted tests for
LineMode’s extra behaviors (getModeInPlotlyand case-insensitive@JsonCreator fromString, including unknown-value rejection).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/rangeSlider/RangeSliderHandleDuplicateFunctionSpec.scala | Pins wire values, enum size, and Jackson round-trip for the range slider duplicate-function enum. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/radarPlot/RadarPlotLinePatternSpec.scala | Pins wire values, enum size, and Jackson round-trip for radar plot line patterns. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineModeSpec.scala | Adds coverage for LineMode wire mapping + Plotly mode translation + fromString behavior + Jackson round-trip. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/histogram2d/NormalizationTypeSpec.scala | Pins wire values, enum size, and Jackson round-trip for histogram2d normalization types. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/hierarchychart/HierarchyChartTypeSpec.scala | Pins wire values, enum size, and Jackson round-trip for hierarchy chart type selection. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/contourPlot/ContourPlotColoringFunctionSpec.scala | Pins wire values, enum size, and Jackson round-trip for contour plot coloring functions. |
| common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/boxViolinPlot/BoxViolinPlotQuartileFunctionSpec.scala | Pins wire values, enum size, and Jackson round-trip for box/violin quartile computation modes. |
💡 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 #6043 +/- ##
============================================
- Coverage 56.93% 56.93% -0.01%
+ Complexity 3026 3025 -1
============================================
Files 1129 1129
Lines 43794 43794
Branches 4743 4743
============================================
- Hits 24936 24934 -2
- Misses 17384 17385 +1
- Partials 1474 1475 +1
*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 | 410 | 0.25 | 22,918/35,195/35,195 us | 🔴 +19.1% / 🔴 +133.5% |
| 🔴 | bs=100 sw=10 sl=64 | 944 | 0.576 | 104,876/134,307/134,307 us | 🔴 +5.4% / 🔴 +24.8% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,090 | 0.665 | 921,644/955,630/955,630 us | ⚪ within ±5% / 🟢 -9.7% |
Baseline details
Latest main 434bcae from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 410 tuples/sec | 449 tuples/sec | 777.62 tuples/sec | -8.7% | -47.3% |
| bs=10 sw=10 sl=64 | MB/s | 0.25 MB/s | 0.274 MB/s | 0.475 MB/s | -8.8% | -47.3% |
| bs=10 sw=10 sl=64 | p50 | 22,918 us | 21,707 us | 12,612 us | +5.6% | +81.7% |
| bs=10 sw=10 sl=64 | p95 | 35,195 us | 29,556 us | 15,070 us | +19.1% | +133.5% |
| bs=10 sw=10 sl=64 | p99 | 35,195 us | 29,556 us | 18,360 us | +19.1% | +91.7% |
| bs=100 sw=10 sl=64 | throughput | 944 tuples/sec | 952 tuples/sec | 988.31 tuples/sec | -0.8% | -4.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.576 MB/s | 0.581 MB/s | 0.603 MB/s | -0.9% | -4.5% |
| bs=100 sw=10 sl=64 | p50 | 104,876 us | 103,340 us | 101,066 us | +1.5% | +3.8% |
| bs=100 sw=10 sl=64 | p95 | 134,307 us | 127,372 us | 107,594 us | +5.4% | +24.8% |
| bs=100 sw=10 sl=64 | p99 | 134,307 us | 127,372 us | 115,830 us | +5.4% | +16.0% |
| bs=1000 sw=10 sl=64 | throughput | 1,090 tuples/sec | 1,102 tuples/sec | 1,019 tuples/sec | -1.1% | +6.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.665 MB/s | 0.673 MB/s | 0.622 MB/s | -1.2% | +6.9% |
| bs=1000 sw=10 sl=64 | p50 | 921,644 us | 908,151 us | 986,982 us | +1.5% | -6.6% |
| bs=1000 sw=10 sl=64 | p95 | 955,630 us | 956,512 us | 1,028,491 us | -0.1% | -7.1% |
| bs=1000 sw=10 sl=64 | p99 | 955,630 us | 956,512 us | 1,058,493 us | -0.1% | -9.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,487.74,200,128000,410,0.250,22918.15,35194.86,35194.86
1,100,10,64,20,2117.71,2000,1280000,944,0.576,104875.58,134307.22,134307.22
2,1000,10,64,20,18346.69,20000,12800000,1090,0.665,921643.60,955629.68,955629.68
Contributor
|
does this PR improve test coverage? @aglinxinyuan. Seems that 300 lines of new tests add coverage on only 1 src line. Let's try to fill up the coverage on the untested code path first. 
|
Contributor
Author
|
Let's skip this for now since it doesn't improve coverage much. |
This was referenced Jul 2, 2026
aglinxinyuan
added a commit
to aglinxinyuan/texera
that referenced
this pull request
Jul 2, 2026
…nd dual-port UDF classes (apache#6057) ### What changes were proposed in this PR? Fill uncovered code paths in three `workflow-operator` classes, selected from the Codecov report. No production-code changes. | File | Codecov before | Missed lines targeted | | --- | --- | --- | | `FilterPredicate.java` | 40.0% | 26 — the remaining `evaluate` branches: GE/LT operators, null-field short-circuit, IS_NULL both ways, boolean (case-insensitive), double, long, and timestamp evaluators, the numeric-string fast path, and the `equals` identity/null/other-type branches | | `BufferedBlockReader.java` | 0% | 51 (whole class) — delimiter splitting, EOF flush (incl. kept-filter bypass), CR terminators and the CRLF single-null artifact, empty fields → null, fields spanning buffer reads, delimiter-at-buffer-start carry, kept-index filtering, `hasNext` block-boundary semantics, `close` | | `DualInputPortsPythonUDFOpDescV2.scala` | 40.0% | 27 — the entire `getPhysicalOp`: worker precondition, env-name resolution (default/blank-reject/trim), single vs parallel wiring, unknown-partition derivation, and the port-1-drives-output schema propagation incl. the duplicate-column rejection | Extends the existing `FilterPredicateSpec` and adds `BufferedBlockReaderSpec` (with a chunked-stream stub to force buffer-boundary paths) and `DualInputPortsPythonUDFOpDescV2Spec`. ### Any related issues, documentation, discussions? Follow-up to the review feedback on apache#6043: prioritize tests that fill uncovered code paths. ### How was this PR tested? - `sbt "WorkflowOperator/testOnly *FilterPredicateSpec *BufferedBlockReaderSpec *DualInputPortsPythonUDFOpDescV2Spec"` — 39 tests, all green - `sbt "WorkflowOperator/Test/scalafmtCheck"` and `sbt "WorkflowOperator/scalafixAll --check"` — clean - CI + Codecov delta to confirm ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context])
aglinxinyuan
added a commit
to aglinxinyuan/texera
that referenced
this pull request
Jul 2, 2026
…ies (apache#6055) ### What changes were proposed in this PR? Fill uncovered code paths in five `workflow-core` tuple/type utility classes, selected from the Codecov report (not spec-name gaps). No production-code changes. | File | Codecov before | Missed lines targeted | | --- | --- | --- | | `Tuple.scala` | 54.1% | 24 — `getField` miss/Attribute overload, `enforceSchema`, equals branches (incl. binary contents / non-Tuple), `getPartialTuple`, `toString`, size/type-mismatch throws, 3-arg builder `add` + null guards, case-class synthetics | | `TupleLike.scala` | 12.8% | 32 — `SeqTupleLike`/`MapTupleLike` `enforceSchema` (positional vs name-based, null fill, extras dropped), `inMemSize` `???`, all seven `TupleLike.apply` overloads, the `NotAnIterable` guard implicit | | `AttributeType.java` | 47.2% | 18 — `getName` ANY branch (`""`), the full `getAttributeType(Class)` mapping incl. primitive/unknown fallback to ANY | | `AttributeTypeUtils.scala` | 71.7% | 20 (+29 partials) — `SchemaCasting`, `tupleCasting`, both `parseFields` overloads, null-inference early-returns, BINARY/ANY inference fallback, LONG/DOUBLE `compare`, unsupported-type throws, forced-parse failures | | `ArrowUtils.scala` | 67.8% | 15 — `appendTexeraTuple`/`setTexeraTuple`/`getTexeraTuple` round-trip (all 7 types + nulls + row indices), parse-failure → null field, `fromAttributeType(null)` throw, unrecognized `texera_type` metadata fallback | Extends the existing `TupleSpec`, `AttributeTypeUtilsSpec`, and `ArrowUtilsSpec` (the operator-module `ArrowUtilsSpec` doesn't attribute coverage to `workflow-core` sources), and adds `TupleLikeSpec` / `AttributeTypeSpec`. Behavior notes pinned along the way: `$attrType` error messages render the lowercase wire name; the `NotAnIterable` iterable-guard implicit is not summoned for multi-iterable varargs under Scala 2.13 inference (pinned by direct invocation). ### Any related issues, documentation, discussions? Follow-up to the review feedback on apache#6043: prioritize tests that fill uncovered code paths. ### How was this PR tested? - `sbt "WorkflowCore/testOnly *TupleSpec *TupleLikeSpec *AttributeTypeSpec *AttributeTypeUtilsSpec org.apache.texera.amber.util.ArrowUtilsSpec"` — 105 tests, all green - `sbt "WorkflowCore/Test/scalafmtCheck"` and `sbt "WorkflowCore/scalafixAll --check"` — clean - CI + Codecov delta to confirm ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context])
aglinxinyuan
added a commit
to aglinxinyuan/texera
that referenced
this pull request
Jul 3, 2026
…icalOp (apache#6056) ### What changes were proposed in this PR? Add unit test coverage for the two largest uncovered files in `workflow-core` per the Codecov report. No production-code changes. | File | Codecov before | Missed lines targeted | | --- | --- | --- | | `PhysicalPlan.scala` | 27.0% | 71 — `addOperator`/`addLink` (incl. schema hand-off + undeclared-port rejection), `removeLink`, `setOperator`, `getPhysicalOpByWorkerId`, `getLinksBetween`, `getOutputPartitionInfo` (all three branches), blocking/dependee link detection, `getDependeeLinksRemovedDAG`, `getNonBridgeNonBlockingLinks` (bridge vs diamond), `maxChains` (maximal-subset filter), `layeredReversedTopologicalOrder` (diamond, parallel edges, empty), plan-level `propagateSchema` | | `PhysicalOp.scala` | 46.2% | 66 — the Java-function `SchemaPropagationFunc` wrapper, all four factory families (`source`/`oneToOne`/`manyToOne`/`local`, both overloads each), `dependeeInputs`/`isInputLinkDependee`, `isPythonBased`/`getCode`, `withPartitionRequirement`/`withDerivePartition`/`withIsOneToManyOp`/`withPveName`, partial-schema propagation gating, `getInputPortDependencyPairs`, `addOutputLink` guards, primary-constructor defaults | New `PhysicalPlanSpec` and `PhysicalOpSpec` in `workflow-core` (no name clash repo-wide); deliberately does not duplicate the `PhysicalOp`/plan surfaces already covered by `WorkflowCoreTypesSpec`. ### Any related issues, documentation, discussions? Follow-up to the review feedback on apache#6043: prioritize tests that fill uncovered code paths. ### How was this PR tested? - `sbt "WorkflowCore/testOnly *PhysicalPlanSpec *PhysicalOpSpec"` — 38 tests, all green - `sbt "WorkflowCore/Test/scalafmtCheck"` and `sbt "WorkflowCore/scalafixAll --check"` — clean - CI + Codecov delta to confirm ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context])
This was referenced Jul 3, 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?
Pin behavior of seven previously-untested visualization option enums in
common/workflow-operator. No production-code changes.RadarPlotLinePatternSpecRadarPlotLinePatternLineModeSpecLineModeHierarchyChartTypeSpecHierarchyChartTypeNormalizationTypeSpecNormalizationTypeRangeSliderHandleDuplicateFunctionSpecRangeSliderHandleDuplicateFunctionContourPlotColoringFunctionSpecContourPlotColoringFunctionBoxViolinPlotQuartileFunctionSpecBoxViolinPlotQuartileFunctionBehavior pinned
@JsonValuestring + constant countsLineModeextrasgetModeInPlotly(lines/markers/lines+markers);@JsonCreator fromStringcase-insensitive + unknown-value rejectionAny related issues, documentation, discussions?
Part of the ongoing
workflow-operatorunit-test coverage effort.How was this PR tested?
sbt "WorkflowOperator/testOnly *RadarPlotLinePatternSpec *LineModeSpec *HierarchyChartTypeSpec *NormalizationTypeSpec *RangeSliderHandleDuplicateFunctionSpec *ContourPlotColoringFunctionSpec *BoxViolinPlotQuartileFunctionSpec"— 16 tests, all greensbt "WorkflowOperator/Test/scalafmtCheck"andsbt "WorkflowOperator/scalafixAll --check"— cleanWas this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])