aglinxinyuan opened a new pull request, #7871: URL: https://github.com/apache/texera/pull/7871
### What changes were proposed in this PR? `SQLSourceOpExecSpec` goes from 30 tests to 40, covering the batch-boundary arithmetic and type dispatch in `SQLSourceOpExec`. Measured with the `FileScanSourceOpExecSpec` suite-name exclusion applied identically to the before and after run — that suite aborts on this machine for pre-existing reasons, and because sbt-jacoco runs unforked a failing test task skips `saveRuntimeData` and emits an all-zero report rather than a partial one. | Metric | Before | After | |---|---|---| | Codecov (fully-covered lines) | 139/163 = 85.3% | **147/163 = 90.2%** | | Missed lines | 4 (230, 231, 364, 519) | **0** | | JaCoCo line-hit | 159/163 = 97.6% | **163/163 = 100%** | | Branch arms | 127 covered / 43 missed | **137 covered / 33 missed** | Tests: spec 30 → 40; the `source.sql.*` package 176 → 186 across 11 suites; the module 2307 → 2317. ### The +8 is not eight lines of behaviour, and the first draft implied it was Of the eight newly fully-covered lines, **five are production-reachable behaviour** — the INTEGER and DOUBLE type dispatch, the progressive arm of the offset guard, the non-auto LONG maximum, and the non-auto unsupported-type throw. The other **three are defensive arms no workflow can reach**: `hasNextQuery`'s unsupported-type throw is unreachable because a successful `open()` admits only INTEGER/LONG/TIMESTAMP/DOUBLE, and two more are similar guards. A reviewer put the split at +6 reachable / +2 defensive. That was closer than the original claim but still not right; the measured split is 5 and 3. ### Verification 22 mutations, **20 killed, 2 survivors.** Every one of the nine mutants the two reviewers reported as surviving was first *reproduced surviving* on the pre-review spec, and eight of the nine now die. **The two survivors are deliberate, and killing them would make the suite worse.** `isLastBatch = nextLowerBound >= upperBound` can be flipped to `>` on both the LONG arm (line 282) and its DOUBLE twin (line 285) and survive all 186 package tests. This was verified by running it, not inferred. The only input that distinguishes the two makes production emit a **duplicate final window** — so a test that killed these mutants would cement that defect. Reported instead. ### Corrections to my own first draft - "10 mutations, all 10 killed, zero survivors" was true of that table, but the table was too narrow to support the bundle's claims — nine further mutants on the very lines the bundle claimed to newly cover were untested, and most of them survived. - The +8/+10 figures reproduce exactly, but were stated without the reachability qualification above. - The conclusion drawn from line 502 was wrong even though its coverage half was right. - Module baselines were off by two: 208 missed lines (96.59%) and 1215 missed branches (51.96%), not 206/1214. - Line counts and test counts in the first draft were stale. - One assertion — `(statement.setLong _).expects(*, *).never()` — carried no information beyond the strict mock itself and is gone, replaced by a positive `expects(1, 2L)` whose strictness does the work. - A CI-wiring claim from the first draft is not restated here, because it was not re-verified in this pass. ### Deliberately not included, with evidence - **Four `case A | B | C | _ =>` arms** (230, 286, 320, 357) keep a dead `ifeq` side: scalac compiles the `_` as a constant before the final branch. - **Ten lines** are sealed-`Option` `None$.equals` checks falling through to `new scala/MatchError`, plus an `$outer` null check and a non-local-return rethrow. - **Line 502's `batchByAttribute.isDefined` false arm is dead**: `initBatchColumnBoundaries` is private with one caller, `open()`, which always assigns `Option(schema.getAttribute(...))`, and `Schema.getAttribute` throws rather than returning null. Widening the private would be a production edit — refused. - **Line 472's `if (limit > 0)` false arm is unreachable**: `generateSqlQuery` returns `None` whenever the limit is `Some(n <= 0)`, so 472 is only reached when the limit is positive. Getting there needs the protected `generateSqlQuery` overridden, i.e. replacing the code under test. No production file is touched — sha256 unchanged from the pre-mutation snapshot, and no stray `test_large_binary.txt` was left behind, since the FileScan suite never ran. ### Any related issues, documentation, discussions? Closes #7868 ### How was this PR tested? ``` sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.sql.SQLSourceOpExecSpec" ``` ``` [info] Total number of tests run: 40 [info] Tests: succeeded 40, failed 0, canceled 0, ignored 0, pending 0 ``` The whole `source.sql.*` package is green at 186 tests across 11 suites, 0 aborted. `Test/scalafmtCheck` passes. ### 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]
