You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "eugenegujing (via GitHub)" <gi...@apache.org> on 2026/08/14 00:13:22 UTC
[PR] test(workflow-compiler): Extend WorkflowCompilerSpec to cover the Python code-generation error path [texera]
eugenegujing opened a new pull request, #7649:
URL: https://github.com/apache/texera/pull/7649
### What changes were proposed in this PR?
`WorkflowCompiler` scans every Python-based physical operator's generated code for the `#EXCEPTION DURING CODE GENERATION:` marker that `PythonOperatorDescriptor` embeds when an operator's `generatePythonCode` throws. On a hit it either appends a `RuntimeException("Operator is not configured properly: ...")` to the caller's error list (the editing-time, lenient path) or throws it immediately when no error list was given (the pre-execution, strict path). `WorkflowCompilerSpec`'s 15 tests never exercised either branch; the only existing test of the marker is the producer-side `PythonOperatorDescriptorSpec`, which asserts the marker is written but not that the compiler reacts to it.
This adds 7 tests to `WorkflowCompilerSpec` (15 → 22). Test-only: no production file is touched, and none of the 15 existing tests is modified — the diff removes exactly three lines, all of them import lines being widened.
Lenient path:
1. `should accumulate a per-operator error when a Python operator's code generation fails` — the error is attributed to the right logical operator, carries the expected message, and compilation **continues** (the rest of the plan still lands in the physical plan and the storage set).
2. `should attribute each Python code-generation failure to its own logical operator` — two failing Python operators with two distinct messages, each keyed to its own id.
3. `should trim the marker's message and report it as a plain RuntimeException`
4. `should report no code-generation error for a well-formed Python operator` — asserts the operator really is still Python-based, so it cannot pass by quietly ceasing to be one.
5. `should not subject non-Python operators to the code-generation check`
Strict path:
6. `in strict mode should throw immediately when a Python operator's code generation failed`
7. `in strict mode should not throw for a well-formed Python operator`
Five of the seven drive the **real shipped `SortOpDesc`**, whose `generatePythonCode` opens with `require(attributes.nonEmpty, ...)` and a per-key `require` (`SortOpDesc.scala:34-38`). A Sort dropped on the canvas and left unconfigured is therefore a genuine, user-reachable route into the marker state, and it conveniently yields two distinct messages — which is what makes test 2's per-operator attribution meaningful. A configured Sort gives the negative control in tests 4 and 7: same operator, same code path, codegen simply succeeds.
Test 3 needs a small test-only fixture (`PaddedFailurePyOp`, ~15 lines, modelled on `PythonOperatorDescriptorSpec`'s `ThrowingPyOp`): no shipped operator raises a whitespace-padded message, and without padding the `.trim` is unobservable. It is also the spec's only source-operator Python case.
Two notes:
- Four assertions hard-code `SortOpDesc`'s exact `require` wording, so rewording those messages in `workflow-operator` will fail these `workflow-compiler` tests. That cross-module coupling is deliberate — it pins the end-to-end string a user actually sees — but it is worth knowing.
- Test 3 also asserts the reported message starts with `java.lang.RuntimeException: `. That prefix comes from `err.toString` at `WorkflowCompiler.scala:66` and is part of what the UI renders today; the assertion carries a comment saying so, so if that pre-existing wart is ever fixed there is one self-explaining test to update.
### Any related issues, documentation, discussions?
Closes #7647
Builds on the specs added by #5019 / #5022 and the module unification in #6143.
### How was this PR tested?
`sbt "WorkflowCompiler/testOnly *WorkflowCompilerSpec"` — 22 tests, all passing (15 pre-existing + 7 new). Running the whole module (`WorkflowCompiler/test`) is green too: 3 suites / 57 tests / 0 failures. `WorkflowCompiler/scalafmtCheck` and `WorkflowCompiler/Test/scalafmtCheck` are clean.
Every new test was mutation-checked, twice and independently: the production check was temporarily broken, the suite re-run, and the file reverted (verified byte-identical afterwards). Highlights, with the tests that are the *sole* killer of a mutant:
| Mutant in `WorkflowCompiler.scala` | Caught by |
|---|---|
| lenient arm appends **and then** throws (abort instead of continue) | 1 only |
| record only the first codegen failure per compile | 2 only |
| drop `.trim` on the captured message | 3 only |
| skip the check for source operators (`&& !isSourceOperator`) | 3 only |
| strict arm swallows instead of throwing (`case None => ()`) | 6 only |
| flip the `errorList` arms (`Some` → throw, `None` → skip) | 1, 6 |
| change the message prefix | 1, 2, 3, 6 |
| `group(1)` → `group(0)` | 1, 2, 3, 6 |
| marker regex changed so it never matches | 1, 2, 3, 6 |
| regex relaxed to `(.*)` (matches every operator) | 1, 2, 3, 4, 6, 7 |
| attribute the error to the wrong logical operator id | 1, 2, 3 |
| build the error but never record it | 1, 2, 3, 6 |
| `RuntimeException` → `IllegalArgumentException` | 3 |
Two mutants survive, both semantically equivalent rather than gaps: reporting the *last* marker match instead of the first (a codegen-failure body is exactly the one-line marker, so first == last), and anchoring the regex with `^` — the latter is in fact the fix for a bug found along the way (below), and all 22 tests stay green under it, so the suite does not over-fit to the current unanchored check.
One more mutant is worth calling out honestly: removing the `isPythonBased` guard entirely does not fail test 5 — it aborts the whole suite, because `PhysicalOp.getCode` throws an `IllegalAccessError` (a `LinkageError`, which is not `NonFatal`) for non-code operators. Test 5 therefore documents the guard's intent rather than detecting its removal. It is kept for that reason, with a self-check that stops it silently becoming a no-op.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(workflow-compiler): Extend WorkflowCompilerSpec to cover the Python code-generation error path [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7649:
URL: https://github.com/apache/texera/pull/7649#issuecomment-5290093480
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7649?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 89.65%. Comparing base ([`4013ca7`](https://app.codecov.io/gh/apache/texera/commit/4013ca74acfefd84d8fc0492fff242f7d9d4442c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`eda8b3c`](https://app.codecov.io/gh/apache/texera/commit/eda8b3cfad1fc76f618f7b140ac16a6d996daf2b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7649 +/- ##
============================================
- Coverage 89.68% 89.65% -0.03%
+ Complexity 4399 4397 -2
============================================
Files 1175 1177 +2
Lines 46979 46996 +17
Branches 5265 5268 +3
============================================
+ Hits 42133 42136 +3
- Misses 3082 3094 +12
- Partials 1764 1766 +2
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.62% <ø> (ø)` | | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.35% <ø> (-0.04%)` | :arrow_down: | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.46% <ø> (ø)` | | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.31% <ø> (ø)` | | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.90% <ø> (ø)` | | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.67% <ø> (-0.04%)` | :arrow_down: | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.57% <ø> (ø)` | | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7649/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.89% <ø> (ø)` | | Carriedforward from [12169c2](https://app.codecov.io/gh/apache/texera/commit/12169c2cfe5f92d0c492ae12d8eb2b53a6fec098?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7649?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(workflow-compiler): Extend WorkflowCompilerSpec to cover the Python code-generation error path [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7649:
URL: https://github.com/apache/texera/pull/7649#issuecomment-5287853943
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@Yicong-Huang`
You can notify them by mentioning `@Yicong-Huang` in a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(workflow-compiler): Extend WorkflowCompilerSpec to cover the Python code-generation error path [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #7649:
URL: https://github.com/apache/texera/pull/7649
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(workflow-compiler): Extend WorkflowCompilerSpec to cover the Python code-generation error path [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7649:
URL: https://github.com/apache/texera/pull/7649#issuecomment-5287893504
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 6 worse · ⚪ 9 noise (<±5%) · 0 without baseline
> Compared against main `12169c2` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/31756625013)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 399 | 0.243 | 24,573/33,854/33,854 us | 🔴 +9.1% / 🔴 +107.1% |
| 🔴 | bs=100 sw=10 sl=64 | 751 | 0.458 | 128,448/166,717/166,717 us | 🔴 -8.8% / 🔴 +54.9% |
| ⚪ | bs=1000 sw=10 sl=64 | 918 | 0.56 | 1,092,061/1,142,894/1,142,894 us | ⚪ within ±5% / 🔴 -11.0% |
<details><summary>Baseline details</summary>
Latest main `12169c2` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 399 tuples/sec | 421 tuples/sec | 774.89 tuples/sec | -5.2% | -48.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.243 MB/s | 0.257 MB/s | 0.473 MB/s | -5.4% | -48.6% |
| bs=10 sw=10 sl=64 | p50 | 24,573 us | 22,520 us | 12,738 us | +9.1% | +92.9% |
| bs=10 sw=10 sl=64 | p95 | 33,854 us | 32,803 us | 16,348 us | +3.2% | +107.1% |
| bs=10 sw=10 sl=64 | p99 | 33,854 us | 32,803 us | 18,848 us | +3.2% | +79.6% |
| bs=100 sw=10 sl=64 | throughput | 751 tuples/sec | 822 tuples/sec | 1,005 tuples/sec | -8.6% | -25.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.458 MB/s | 0.502 MB/s | 0.613 MB/s | -8.8% | -25.3% |
| bs=100 sw=10 sl=64 | p50 | 128,448 us | 118,379 us | 100,970 us | +8.5% | +27.2% |
| bs=100 sw=10 sl=64 | p95 | 166,717 us | 159,553 us | 107,605 us | +4.5% | +54.9% |
| bs=100 sw=10 sl=64 | p99 | 166,717 us | 159,553 us | 116,429 us | +4.5% | +43.2% |
| bs=1000 sw=10 sl=64 | throughput | 918 tuples/sec | 915 tuples/sec | 1,030 tuples/sec | +0.3% | -10.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.56 MB/s | 0.558 MB/s | 0.629 MB/s | +0.4% | -11.0% |
| bs=1000 sw=10 sl=64 | p50 | 1,092,061 us | 1,100,911 us | 991,433 us | -0.8% | +10.1% |
| bs=1000 sw=10 sl=64 | p95 | 1,142,894 us | 1,145,303 us | 1,036,668 us | -0.2% | +10.2% |
| bs=1000 sw=10 sl=64 | p99 | 1,142,894 us | 1,145,303 us | 1,070,470 us | -0.2% | +6.8% |
</details>
<details><summary>Raw CSV</summary>
```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,501.41,200,128000,399,0.243,24572.83,33854.18,33854.18
1,100,10,64,20,2664.87,2000,1280000,751,0.458,128447.94,166716.57,166716.57
2,1000,10,64,20,21798.20,20000,12800000,918,0.560,1092061.23,1142894.49,1142894.49
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org