The GitHub Actions job "Required Checks" on texera.git/gh-readonly-queue/main/pr-7207-c37f6ab7c68f15e7a1aa283d0c29b84e11f1ed6d has succeeded. Run started by GitHub user aglinxinyuan (triggered by aglinxinyuan).
Head commit for run: 2f77d5ab93f94c8e9b33195f805a35e63c52f970 / Kary Zheng <[email protected]> test(workflow-operator): route Python tests to amber-integration (#7207) ### What changes were proposed in this PR? The `amber` job runs the `WorkflowOperator` test suite and installs no Python packages. A test that *executes* an operator's generated template, rather than only `py_compile`-ing it, therefore fails its dependency probe and cancels — and a cancellation is neither a pass nor a failure, so the suite still reports "All tests passed" and the gap leaves no trace in the build. This gives `WorkflowOperator` the split `amber` already has: - **Tag** — `IntegrationTest` under `common/workflow-operator/src/test`. amber's cannot be reused: it lives in `amber/src/test/integration`, and amber depends on this module rather than the reverse. - **Filter** — `common/workflow-operator/build.sbt` reads the same `AMBER_TEST_FILTER`. The `amber` job already sets it on the step that invokes `WorkflowOperator/jacoco`, so no workflow change is needed for the exclusion to take effect. - **Job** — `"WorkflowOperator/test"` added to `amber-integration`, which already runs `integration-only` with `amber/requirements.txt` and `amber/operator-requirements.txt` installed (pandas 2.2.3, plotly 5.24.1). - **Shared logic** — the env-var-to-ScalaTest-args mapping moves to `project/TestFilters.scala`, beside the build's other shared helpers (`AddMetaInfLicenseFiles.scala`, `JdkOptions.scala`); each module passes its own env var and tag, the two a caller has to match to the workflow and to the annotation. One behavior changes, and amber inherits it: a value that is neither of the two now fails at project load, where the inline code fell through to running everything. Nothing sets a third value, so what this catches is a typo that would otherwise leave a job running the whole suite while reporting the subset it asked for. - **First user** — `PythonCodeRawInvalidTextSpec` gains a tagged case asserting that pandas and plotly import in the interpreter it already resolves: `py_compile` only parses the emitted code, while running it needs the packages it imports. Being tagged, it also exercises the routing, and it turns a missing install in `amber-integration` into a failure rather than the silent cancellation above. Elsewhere a bare interpreter is a local-setup fact, so it cancels instead. The second half addresses the scaling problem in the same issue: testing operators one at a time does not scale when each one costs a spawn. - **Batching** — the `py_compile` check spawned `python -I -S -B -m py_compile` once per `PythonOperatorDescriptor`, 117 of them serially, where the interpreter boot is the entire cost and the compile is under a millisecond. It now goes through `PythonWorkerPool`: a worker launched once with the same `-I -S` isolation, serving many sources over a line-delimited JSON protocol. What the check accepts is unchanged — `compile(source, path, "exec")` is what `py_compile` does before writing a `.pyc`, and raises the same SyntaxError — and any worker the pool cannot give out or keep falls back to the spawn, so behavior is never worse than before: one that would not start, would not report ready, or died mid-job all leave through the same exception, and the check counts the descriptors that took the spawn into its summary so a run the pool served none of does not read as a green pooled run. `TEXERA_TEST_PYTHON_WORKER=0` selects the old path outright. - **Parallelism** — the descriptors are fanned out across the pool's workers, four at a time; the fan-out's executor is sized to the pool's cap, so nothing runs past it. Together: 1119 ms to 310 ms. That cap is per sub-pool, keyed by script, arguments and environment, and is env-overridable, so it is the fan-out's own sizing that bounds this — not a ScalaTest parameter, which no suite in the module currently parallelizes under anyway. - **Where the pool lives** — this module's test scope, not beside its caller. The tests this PR unblocks execute generated templates against pandas and plotly, where a spawn costs 260-310 ms, mostly the imports, dwarfing the ~4 ms a job computes for. Without a shared seam each such test hand-rolls a driver, a stdout protocol, a timeout and an interpreter probe — the runtime test in #7149 already does. Other modules reach it through a `test->test` dependency. ### Any related issues, documentation, discussions? Fixes #7186. Found while reviewing #7149, whose runtime test is the case cancelling today; tagging it is a one-line follow-up once this is in, and converting its hand-rolled driver to the pool is the natural second one. The pooled-worker design, and the measurements behind those figures, are in #6975. ### How was this PR tested? - `amber` is unchanged by the extraction: `show WorkflowExecutionService/Test/testOptions` under all three env values gives arguments byte-identical to its previous inline code — `-l <tag>`, `-n <tag>`, nothing when unset. `WorkflowOperator` was checked the same way and yields its own tag in those positions. - The new case, run as `WorkflowOperator/testOnly *PythonCodeRawInvalidTextSpec`: passes under `integration-only` (1 test selected); is excluded under `skip-integration` while the spec's two existing tests still run; and, pointed at a bare interpreter, fails under `integration-only` and cancels with no filter set. - The pooled check agrees with the spawn it replaces: 117/117 descriptors pass either way, and `TEXERA_TEST_PYTHON_WORKER=0` flips between them. Timed with `-oD`: 1119 ms spawning, 310 ms pooled. - Red-checked, since a green suite only proves the two paths agree on valid code: appending a syntax error to every generated source takes the pooled path to `ok=0/117` and fails the test, so it still detects what it is there to detect. - Each way a worker could escape the pool's borrow/return discipline was reproduced before being fixed, and `PythonWorkerPoolSpec` keeps a case on each: an interrupted caller used to leave a worker neither returned nor discarded, and the case that refills every slot and then asks for that many jobs again spins at the cap until its bound without the fix; a first line that is not the protocol used to raise `JsonParseException` past the caller's fallback, and the fixture now has a mode that writes one; an interpreter that cannot be started arrives as the pool's own exception rather than `ProcessBuilder`'s. - The fallback is never worse than the spawn it replaces, checked end to end: with the pooled path pointed at an interpreter that does not exist, all 117 descriptors fall back, all 117 still pass, and the summary reports 117 fallbacks. - `WorkflowOperator/test` under `skip-integration`: 286 suites, 2163 tests, none failed. - `sbt scalafmtCheckAll` is clean, and `WorkflowOperator`'s `Compile` and `Test` scalafix checks pass. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --------- Signed-off-by: Kary Zheng <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]> Co-authored-by: Xinyuan Lin <[email protected]> Co-authored-by: Yicong Huang <[email protected]> Report URL: https://github.com/apache/texera/actions/runs/31789109750 With regards, GitHub Actions via GitBox
