aglinxinyuan opened a new pull request, #7883:
URL: https://github.com/apache/texera/pull/7883
### What changes were proposed in this PR?
Two operator-evaluation files whose gaps were entirely branch-arm partials
on already-executed lines. 33 tests → 44.
Measured with a full-module `WorkflowOperator/jacoco`, one fresh sbt JVM per
run, `rm -rf` on the jacoco dir between them, and the same suite-name exclusion
on both sides — `FileScanSourceOpExecSpec` aborts in this worktree on a
pre-existing Windows file lock, and since sbt-jacoco runs unforked a failing
test task yields *no report directory at all*, not an all-zero one.
| File | Codecov | Branch arms |
|---|---|---|
| `DictionaryMatcherOpExec.scala` | 44/50 = 88.0% → **49/50 = 98.0%** | 8
missed → **1** |
| `FilterPredicate.java` | 54/60 = 90.0% → **56/60 = 93.3%** | 6 missed →
**3** |
| **bundle** | 98/110 = 89.1% → **105/110 = 95.5%** | +10 arms |
JaCoCo line-hit was already 100% on the Scala file and 59/60 on the Java one
— every gap was a partial arm on a line that already executed, which is exactly
the case Codecov penalises and line-hit hides.
**This is +7 lines, and I would rather say so than dress it up.** What earns
the PR is the behaviour underneath.
### The behaviour was much less pinned than the percentages suggested
Two findings that are corrections to my own first draft, both material:
- **`FilterPredicate`'s comparison switch was barely constrained.** The
first draft called the file "essentially exhausted" — true of coverage, false
of behaviour. Four of its six comparison arms could be swapped for a
neighbouring arm and stay green against all 2314 module tests. Those are now
pinned.
- **`URL_STOP_WORDS_SET` had no proof it did anything.** The first draft
claimed its new test was "the first proof anywhere in the repo that that list
does anything at all". That was true of the single entry `"www"` and nothing
else — truncating the whole list to `List("www")` survived. The test is now a
loop that pins every entry.
### Verification
26 mutations re-derived from scratch, **25 killed by a named test with its
exact failure message, 1 equivalent mutant.**
The first draft reported no survivors. **Eleven semantic mutants were alive
on it** — five on `FilterPredicate`'s comparison switch, five on
`DictionaryMatcherOpExec` (comma delimiter, field case-folding, null handling),
and one that had slipped past both stop-word tests. All eleven now die, each
re-credited to the test that kills it.
The one mutant left alive drops a field from `hashCode`; killing it would
require asserting a specific hash value, so it is recorded as equivalent rather
than counted as a survivor.
### Corrections, including to a reviewer
- A reviewer suggested an operand-swap variant (`Objects.equals(attribute,
that.value)`) as a survivor for these fixtures. **Wrong** — I ran it, and it is
killed at HEAD by two pre-existing tests, because the attribute name and the
compared value are distinguishable in those fixtures.
- The first draft's fallback estimate ("drop that one test and the bundle
falls to +6 lines / +8 arms") was off by one arm; the real contribution of that
test is 1 line and 3 arms.
- Its module-wide branch figure was off by one: the baseline is 1214 → 1203,
of which −10 comes from these two files and −1 from an unrelated file drifting.
- Its claim that two lines "report as fully covered despite never-executed
instructions" was true of one line, at baseline only.
- A judgement call flagged in the first draft no longer applies: the test no
longer pins `intercept[MatchError]`, so it does not cement scalac's exception
choice as the misconfiguration contract.
### One landmine worth recording for whoever touches this spec next
`DictionaryMatcherOpExecSpec`'s `opExec` is a shared `var`, and its final
test ("close properly") asserts `tokenizedDictionaryEntries == null` on
whatever the *previous* test left behind. That only holds because the preceding
test uses `SUBSTRING`: `close()` clears `tokenizedDictionaryEntries` but never
nulls it — only `dictionaryEntries` and `luceneAnalyzer` are nulled. Inserting
any `CONJUNCTION_INDEXBASED` test immediately above it makes that pre-existing
test fail. Three of the new tests are `CONJUNCTION`, so all new tests are
appended *after* it.
### Deliberately not included
`DictionaryMatcherOpExec` line 62 keeps one structurally unreachable arm, so
49/50 is its ceiling.
`FilterPredicate` lines 66, 80, 130 and 144 stay open and all four are dead:
66 and 80 would need reflection into the private static `evaluateFilter` or a
production seam, and 130/144 are null ternaries the caller already guards.
Widening any of them would be a production edit — refused.
No production file is touched. Both files are md5-identical to the
pre-mutation snapshots, and no stray `test_large_binary.txt` was left behind.
### Any related issues, documentation, discussions?
Closes #7882
### How was this PR tested?
```
sbt "WorkflowOperator/testOnly
org.apache.texera.amber.operator.dictionary.DictionaryMatcherOpExecSpec
org.apache.texera.amber.operator.filter.FilterPredicateSpec"
```
```
[info] Total number of tests run: 44
[info] Tests: succeeded 44, failed 0, canceled 0, ignored 0, pending 0
```
Test counts read from the JUnit XML `tests=` attribute rather than counted
by eye: `DictionaryMatcherOpExecSpec` 11 → 20, `FilterPredicateSpec` 22 → 24.
`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]
