test(verify): run every operator both ways and report what it did by kz930 · Pull Request #8362 · apache/texera

kz930 · 2026-09-02T23:31:55Z

What changes were proposed in this PR?

The runner discovers the operators that implement the trait, configures each
from its schema, runs it through the engine and through its generated script,
and compares. An operator that cannot be run is reported as a row with the
reason rather than passed over, since an operator missing from the report is
a fact the report has to carry.

OperatorBehaviorSpec is what turns that into tests: one per operator per
configuration its schema offers, named so a single operator can be run alone.

Any related issues, documentation, discussions?

Part of #8325, 23 of 27; that issue lists the set in order.

Closes #8425, the task this change is the whole of.

How was this PR tested?

The tests in this change cover it. The whole set is exercised together once the last piece lands: every operator run through the engine and through its generated script, and the two answers compared.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

The runner discovers the operators that implement the trait, configures each from its schema, runs it through the engine and through its generated script, and compares. An operator that cannot be run is reported as a row with the reason rather than passed over, since an operator missing from the report is a fact the report has to carry. `OperatorBehaviorSpec` is what turns that into tests: one per operator per configuration its schema offers, named so a single operator can be run alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-02T23:32:13Z

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

No candidates found from git blame history.

codecov-commenter · 2026-09-02T23:35:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.00%. Comparing base (7976744) to head (5dab44f).
⚠️ Report is 35 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8362      +/-   ##
============================================
- Coverage     94.11%   94.00%   -0.11%     
- Complexity     4811     4862      +51     
============================================
  Files          1197     1204       +7     
  Lines         48813    49076     +263     
  Branches       5906     5948      +42     
============================================
+ Hits          45939    46135     +196     
- Misses         1420     1465      +45     
- Partials       1454     1476      +22

Flag	Coverage Δ		*Carryforward flag
access-control-service	`80.18% <ø> (-0.82%)`	⬇️
agent-service	`99.32% <ø> (ø)`		Carriedforward from f43ef55
amber	`90.08% <ø> (ø)`		Carriedforward from f43ef55
computing-unit-managing-service	`77.14% <ø> (+3.47%)`	⬆️
config-service	`87.12% <ø> (+0.12%)`	⬆️
file-service	`83.65% <ø> (-4.27%)`	⬇️
frontend	`96.86% <ø> (ø)`		Carriedforward from f43ef55
notebook-migration-service	`83.73% <ø> (+4.42%)`	⬆️
pyamber	`98.47% <ø> (ø)`		Carriedforward from f43ef55
workflow-compiling-service	`77.19% <ø> (ø)`		Carriedforward from f43ef55

*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.

Three examples where one carries the point, and a cross-reference the reader can follow without being told to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…erator's issue The row pointed at the issue for Substring Search and Unnest String, which this operator was never part of. That issue closes with the change that answers those two, and this one still raises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The row said the platform raised on an empty cell. The operator answers one now, so the variant runs like every other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…re in Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Both had been withheld for drawing a different picture on every run, which stopped being true once their placement was seeded. The registration is inert until the operator implements the trait, so it can sit here rather than making the change that gives them their generator depend on this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Which tier an operator lands in is the one thing the per-operator runs do not check, so it is what this spec is for. Every visualization, the scorer, an estimator and an advanced trainer each state where they route and why that is not the tier they used to be in. All of them land before this change does, so the assertions hold when it merges rather than needing to be added back afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

One assertion per line, like every other test here. The loop it replaced was the only one of its kind in the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

One said the coverage report prints the withheld runs together. It no longer prints them, so what is left is the reason that does not depend on a reader: one table means the next exemption has an obvious home. The other pointed at `TransformHandler.unfillableVariants`, which does not exist. What it meant is `nullsKeepFilled`, the one thing a handler still says about its own table rather than about its operator. `withheldRunsFor` was public for the report that read it. Its only caller now is beside it, so it says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2

The transform verification runner looks good.

The configuration generator produces it; this is the label a withholding would have to name. A paragraph the one below it already carried goes with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2

Reviewed alongside the related export and verification PRs. These findings are based on code inspection and focused Python checks, not a full Scala suite run.

Every other verify spec runs one operator's fragment with the harness binding its placeholders, so a fault that only appears once fragments share a script and a directory has nowhere to show. Three of those are pinned here: a column named after a placeholder, two charts writing one file, and a branch that ends the process the other branches still need. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

An alternate table can now be written with its holes. The nulls case runs the base config, so a hole only ever landed in a column the operator carries through, never in the one a pin points it at, and the question of what an operator does with a holed column it READS had nowhere to be asked. Unnest String and Substring Search are pointed at the holed integer and boolean columns, and Word Cloud at a numeric one, which its type rule only warns about. The connected-plan spec gains the two faults a single fragment cannot show: a CSV column's declared type reaching an operator a hop away, and a timestamp that only becomes a real datetime after a cast. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

One more run per operator, on the same columns emptied. It asks a different question from the nulls run: that table still has a value in every column somewhere, so code reading a column's range or its quantiles still finds one, while here there is nothing to read and an operator that assumed otherwise raises rather than passing the emptiness through. The estimators are withheld: sklearn refuses to fit on no rows, and where one does fit, no row is left to predict on, so two models cannot be told apart either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

No withheld row names the hostile column run, and no row builds its label through the helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kz930 · 2026-09-11T01:18:52Z

@carloea2 the finding is fixed: a connected plan is now translated and run end to end. Would you take another look when you have a moment?

carloea2

The translated chain and branch tests now run through the real translator. Looks good.

carloea2

Visualization verification compares only one Plotly figure. Bullet and Gauge can produce one figure per input row, but standalone writes only the first figure. A wrong second figure can pass. Please compare every produced figure and add a multirow test.

carloea2

The only full plan Union test gives both inputs the same file. Wiring the first input twice and dropping the second still passes. Please use distinct input data and assert a value unique to each port.

carloea2

The full plan tests only create links from output port 0. A Split or If workflow can wire output port 1 incorrectly and every connected plan test still passes. Please add a connected plan that uses both output ports and verify each branch separately.

The union plan gave both of its inputs the same file, so a plan that wired one upstream twice and dropped the other read the same rows and passed. Each port now reads a file of its own, and the assertion is on the union's own frame rather than on everything the run printed: a dropped upstream is still a leaf of the plan, and printing that leaf puts its rows in stdout anyway. Every plan here also read its upstream's port 0, so a translator that handed each downstream the first frame its upstream produced would pass all of them. The new plan wires both of a split's output ports into branches of their own and reads each branch separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A bullet or gauge chart draws a figure per input row: both paths put all of them on the page they render, but the exported script writes only the first to its output.json, and that file was the side being compared. Nine of the ten figures a run drew went unread, so a wrong one passed. Those two operators are now compared against the page, which is the side holding every figure. The ten agreed already, so this closes the hole rather than a defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kz930 · 2026-09-18T23:36:00Z

All three fixed: each union port now reads its own file and the assertion is on the union's own frame, a new plan wires both of a split's output ports and reads each branch separately, and the Plotly comparison now reads every figure a run drew rather than the first. That last one lands in #8359 and #8509, where compare.py and the comparator live.

The epilogue writes a declared-integral column as an integer, the way the engine's writer does, and the schemas to go by are the ones this run already has in hand from the Texera path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2

The connected plan checks now cover the separate output routes and distinct source inputs. Looks good.

github-actions Bot added feature platform Non-amber Scala service paths labels Sep 2, 2026

github-actions Bot assigned kz930 Sep 2, 2026

kz930 and others added 9 commits September 2, 2026 16:51

docs: shorten the knob-scope note to the cases that earn their place

e7d36c7

Three examples where one carries the point, and a cross-reference the reader can follow without being told to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test(verify): run Regex on the nulls variant now that it answers them

6734650

The row said the platform raised on an empty cell. The operator answers one now, so the variant runs like every other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test(verify): leave the per-operator spec to the change its callees a…

1ba3ca8

…re in Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

style: drop the trailing comma the removed rows left behind

94c65c5

style: assert the visualizations the way the rest of the file does

97f71dd

One assertion per line, like every other test here. The loop it replaced was the only one of its kind in the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2 approved these changes Sep 4, 2026

View reviewed changes

kz930 marked this pull request as draft September 4, 2026 17:12

test(verify): name the hostile-column run

f1e3d14

The configuration generator produces it; this is the label a withholding would have to name. A paragraph the one below it already carried goes with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kz930 force-pushed the feat/verify-run-every-operator branch from e450a54 to f1e3d14 Compare September 9, 2026 06:50

carloea2 reviewed Sep 9, 2026

View reviewed changes

Comment thread ...e/src/test/scala/org/apache/texera/amber/translator/verify/TransformVerificationRunner.scala

kz930 force-pushed the feat/verify-run-every-operator branch from a8a3653 to eb8b5aa Compare September 9, 2026 23:36

kz930 force-pushed the feat/verify-run-every-operator branch from eb8b5aa to b052093 Compare September 9, 2026 23:46

kz930 and others added 3 commits September 10, 2026 13:15

test(verify): drop a run kind and a label helper nothing names

5dab44f

No withheld row names the hostile column run, and no row builds its label through the helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

carloea2 approved these changes Sep 18, 2026

View reviewed changes

carloea2 suggested changes Sep 18, 2026

View reviewed changes

kz930 and others added 2 commits September 18, 2026 15:51

carloea2 approved these changes Sep 19, 2026

View reviewed changes

kz930 mentioned this pull request Sep 19, 2026

feat(visualization): export the tables, ranges and embedded documents #8366

Draft

Navigation Menu

test(verify): run every operator both ways and report what it did - #8362

test(verify): run every operator both ways and report what it did#8362
kz930 wants to merge 18 commits into
apache:mainfrom
kz930:feat/verify-run-every-operator

kz930 commented Sep 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 2, 2026

Uh oh!

codecov-commenter commented Sep 2, 2026 •

edited

Loading

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

Uh oh!

kz930 commented Sep 11, 2026 •

edited

Loading

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

carloea2 left a comment

Uh oh!

kz930 commented Sep 18, 2026

Uh oh!

carloea2 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kz930 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this PR?

Any related issues, documentation, discussions?

How was this PR tested?

Was this PR authored or co-authored using generative AI tooling?

Uh oh!

github-actions Bot commented Sep 2, 2026

Automated Reviewer Suggestions

Uh oh!

codecov-commenter commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kz930 commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

kz930 commented Sep 18, 2026

Uh oh!

carloea2 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kz930 commented Sep 2, 2026 •

edited

Loading

codecov-commenter commented Sep 2, 2026 •

edited

Loading

kz930 commented Sep 11, 2026 •

edited

Loading