aglinxinyuan opened a new pull request, #7863:
URL: https://github.com/apache/texera/pull/7863
### What changes were proposed in this PR?
`CollaborationResourceSpec` goes from 16 tests to 28, covering the parts of
`CollaborationResource` that decide who holds the editing lock.
Measured with two `WorkflowExecutionService/jacoco` runs, one fresh sbt JVM
each, `rm -rf` on the jacoco dir before each, and an identical FQCN suite-name
filter in both — never a `-z` phrase filter. `TEXERA_SERVICE_LOG_LEVEL=WARN` to
match CI. The "before" spec came from `git show HEAD:<path>`.
| Metric | Before | After |
|---|---|---|
| Codecov (fully-covered lines) | 67/87 = 77.0% | **74/87 = 85.1%** |
| JaCoCo line-hit | 4 of 87 missed | **0 missed = 100%** |
| Branch arms | 27 of 74 missed (63.5% covered) | **15 of 74 missed
(79.7%)** |
The before run reproduces the reported "4 missed + 16 partial of 87, 77.0%,
63.51% branch" figure for figure, so this is the line map Codecov sees.
New tests cover a session that re-registers on a different wid, the lock
holder not handing the lock to a read-only peer, exactly one of several
writable peers receiving it, and sessions sitting on a different workflow being
ignored.
### Verification
18 mutations, **17 killed, 1 alive.** Nine of the seventeen are killed by
exactly one test each.
**The first draft claimed "no surviving mutants". That was wrong, and six
survivors were confirmed empirically** — each re-run against the pre-review
spec and each coming back `24 succeeded, 0 failed`. Five are now dead; the
sixth is reported below. Four of the six existed because a test asserted
something other than what its name claimed:
- A test named "hand the lock to a remaining **writable** peer" pinned
nothing about writability: both sessions authenticated as the same uid, so the
privilege predicate was unobservable. Fixed by seeding a genuinely read-only
second peer and asserting it does *not* receive the lock.
- Flipping the once-only granting guard survived, because a single peer in
the bucket can only prove the latch *starts* false. That row is now correctly
described as evidence for the latch's initial value, and once-only granting is
pinned by a separate multi-peer test.
- "Should leave a null holder sentinel alone" pinned only Scala's null-safe
`==` — every assertion also passed on the inverted branch. Rewritten.
- `peer.getId shouldBe "s2"` was a ScalaMock echo of `(() =>
session.getId).expects().returning("s2")`, asserting the fixture rather than
production. Removed.
**The surviving mutant, stated plainly:** substituting
`sessionIdWIdMap(sessId)` for the passed `wId` in the read-only check at line
188 survives all 28 tests. It is equivalent over every state the class's own
bookkeeping invariant permits, since the registration paths write that wid
themselves. Killing it would require constructing a state the class cannot
reach.
### This repair pass added zero coverage, and that should not be misread
The pre-review 24-test spec already measured 74/87 = 85.1% with 15/74 arms
missed; the 28-test spec measures byte-identically the same. The four added
tests earn no tracked lines because the hand-off loop body compiles to
`ACC_SYNTHETIC` `$anonfun` methods that JaCoCo's `SyntheticFilter` drops —
`jacoco.xml` has no `<line>` entry for 188-195 at all. They buy mutation
strength, not lines.
### Deliberately not included
13 lines remain partial and none were chased: logging guards, a lazy-val
bitmap, `MatchError` arms, dead short-circuit arms on line 122, and
null-`Session.getId` arms on 135 and 181. None of them is a surviving mutant —
they are structurally unreachable.
Stated as a gap rather than a claim: the `CommandRequest` /
`RestoreVersionRequest` fan-out loops got no mutations in either pass. Their
bodies are synthetic `$anonfun` and earn zero coverage lines, and scope was not
expanded into them.
### Reported, not pinned
Two defects are named in the spec rather than asserted: the
`AcquireLockRequest` null-sentinel handling, and a stale-bucket leak. A
pre-existing test already cements the first as intended behaviour, which is
worth knowing before anyone changes it.
No production file is touched — `git diff -- '*/src/main/*'` was checked
after all 24 mutation reverts and again at the end, 0 bytes each time, with the
production sha256 byte-identical to the pre-mutation snapshot.
### Any related issues, documentation, discussions?
Closes #7861
### How was this PR tested?
```
sbt "WorkflowExecutionService/testOnly
org.apache.texera.web.resource.CollaborationResourceSpec"
```
```
[info] Total number of tests run: 28
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 28, failed 0, canceled 0, ignored 0, pending 0
```
The test-report XML independently counts 28 `testcase` entries with no
`failure` or `error`. `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]
