aglinxinyuan opened a new pull request, #7899: URL: https://github.com/apache/texera/pull/7899
### What changes were proposed in this PR? Four operator descriptors. 36 tests → 47. Measured with a full-module `WorkflowOperator/jacoco`, one fresh sbt JVM per run, and the same suite-name exclusion on both sides (`FileScanSourceOpExecSpec` aborts inside a git worktree — a worktree's `.git` is a file, so it throws `RepositoryNotFoundException` in `beforeAll` — and since sbt-jacoco runs unforked, a failing test task yields an all-zero report rather than a partial one). | File | Codecov | Missed arms | |---|---|---| | `SQLSourceOpDesc.scala` | 88.9% → **100%** | 11 → **0** | | `RUDFSourceOpDesc.scala` | 80.6% → **100%** | 3 → **0** | | `MachineLearningScorerOpDesc.scala` | 84.8% → **100%** | 4 → **0** | | `ECDFPlotOpDesc.scala` | 85.4% → **87.5%** (its ceiling) | 15 → **12** | | **bundle** | 173/203 = 85.2% → **192/203 = 94.6%** | 48 → **27** | JaCoCo line-hit goes 199/203 → 203/203. **`TimeSeriesOpDesc` was in the original scope and is deliberately absent.** It is worth exactly zero: all five of its partials are Scala's null-safe comparison against a String *literal*, where one arm requires the constant itself to be null. A probe setting every field to null left it unchanged at 23/28. And the null path is a crash rather than a behaviour — `dropnaCols.map(c => pyb"$c")` NPEs inside the `pyb` macro on Base64 of a null string. Pinning it would cement junk. ### Verification 28 mutations, **27 killed, 1 equivalent survivor.** The first draft reported no survivors. Two adversarial reviewers demonstrated **8 survivors** between them, and every one that could be reproduced was real. All are now dead. Representative repairs, each proven by the mutation it now kills: - The JDBC type map pinned 9 of 19 keys; the remaining ten are now covered (`TINYINT`→INTEGER, `FLOAT`/`REAL`→DOUBLE, `BIT`→BOOLEAN, and the rest). - Three error-message assertions used `endWith`, leaving the half of the message that names the failing operator unpinned. They are now exact equalities against the concrete subclass name the spec itself controls. - ECDF argument assertions were presence-only; they now check payloads through the spec's existing Base64 helper, and three index-arithmetic assertions pin the emitted body's order (`pd.to_numeric(` before `px.ecdf(` before `plotly.io.to_html(`). - The scorer's actual and predicted columns were interchangeable; the loose whole-code `include` checks are now line-anchored. **The survivor:** deleting `.withParallelizable(false)` from the else arm of RUDF's worker-count guard. `PhysicalOp.sourcePhysicalOp` already defaults it, so nothing observable changes — an equivalent mutant, not a coverage gap. ### Two reviewer suggestions were refused - One proposed deleting the RUDF single-worker test as vacuous. The premise was half right — the `withParallelizable(false)` call really is unobservable — but the suggested fix would have made the suite **weaker**, since the test pins more than that one call. - Its optional follow-up asked for a production deletion, which this pass forbids. Recorded as a limitation rather than actioned. A collateral claim about the builder's own harness was also left unasserted rather than repeated, on the grounds that it could not be verified from the state the worktree was handed over in. ### A nondeterminism source found along the way The module-wide branch total does not move by exactly the amount these four files account for, and the residual is **not** measurement noise in this bundle: it is `IntervalJoinOpExec.scala`, whose covering spec uses an **unseeded RNG**, so its branch count drifts run to run. That is worth knowing because it makes module-level branch totals unquotable as evidence for any PR touching this module. Seeding that spec is already part of #7800. Same-input reproducibility is otherwise good: the final `jacoco.xml` is byte-identical to a reviewer's independently produced report. ### Deliberately not included, with evidence - **`ECDFPlotOpDesc` cannot exceed 42/48.** `javap` on `createPlotlyFigure` shows the three-jump null-safe form, and the `ifnull`-taken arm needs the literal `"sum"` to be null. A probe moved those lines from `cb=3` to `cb=5` with `mb` unchanged — 8 arms, zero lines. Two further lines would need a `hashCode` collision with `"count"` or `"sum"` that is not equal to it: constructible in principle, padding in practice. - **The scorer's last two lines are reachable only through an erasure violation.** The LUB of the two metric list types erases to `java.lang.Enum`, so the lambda bridge's checkcast admits a foreign enum — `List(java.time.DayOfWeek.MONDAY).asInstanceOf[…]` does reach the `case _` throw and takes the file to 33/33. No production route exists, because Jackson rejects an unknown enum name first, so it is excluded. - **The six `require` message strings are lifted to `$anonfun$querySchema$1..6`** and dropped by `SyntheticFilter`; their source lines already read as covered because the closure allocation is attributed to the enclosing method. - One file's headline 100% is worth qualifying: the `foldLeft` body making its only schema-typing decision is a `$anonfun`, so it is not in the tracked denominator either way. Every spec in the bundle carries a JSON round-trip test, per house convention. No descriptor subtype is defined in any spec, because `PythonCodeRawInvalidTextSpec` instantiates every subclass by reflection and a test-only subtype would break it. No production file is touched, and no stray `test_large_binary.txt` was left behind. ### Any related issues, documentation, discussions? Closes #7898 ### How was this PR tested? ``` sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.sql.SQLSourceOpDescSpec org.apache.texera.amber.operator.udf.r.RUDFSourceOpDescSpec org.apache.texera.amber.operator.machineLearning.Scorer.MachineLearningScorerOpDescSpec org.apache.texera.amber.operator.visualization.ecdfPlot.ECDFPlotOpDescSpec" ``` ``` [info] Total number of tests run: 47 [info] Tests: succeeded 47, failed 0, canceled 0, ignored 0, pending 0 ``` `Test/scalafmtCheck` and `Test/scalafix --check` both pass. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
