aglinxinyuan opened a new issue, #7902:
URL: https://github.com/apache/texera/issues/7902

   ### Task Summary
   
   Two small files with no spec at all:
   
   | File | Coverage |
   |---|---|
   | `amber/.../messaginglayer/WorkerTimerService.scala` | 80% — 3 missed + 2 
partial of 25 lines |
   | `amber/.../dataset/utils/DatasetStatisticsUtils.scala` | 70.6% — 5 missed 
of 17 lines |
   
   `ConfigParserUtil` looks like a third candidate but already has a spec on 
main and needs nothing.
   
   Traps, several of which are ways to write a test that pins nothing:
   
   1. **Do not manufacture a precondition by writing production state from the 
test.** Setting `service.adaptiveBatchingHandle = None` to reach a resume path 
makes the assertion that follows vacuous — it asserts the value the test just 
wrote. Reach the precondition through the production API instead.
   2. **The scheduler interval is easy to pin self-referentially.** A test that 
reads `ApplicationConfig.adaptiveBatchingInterval` on both sides asserts 
nothing about which config key production uses. Pinning it needs the value the 
constructor captured, which is reachable only by reflection on a private final 
field — the only test-only seam available.
   3. **Substituting a different config key is only partly discriminable.** Any 
`Int`-typed accessor whose value differs is caught by value; one that happened 
to hold the same value would survive. Worth stating rather than claiming the 
key is pinned.
   4. **`Cancellable.cancel()`'s Boolean is easy to leave always-true**, which 
hides a whole mutant class. Parameterise the test helper so the false path is 
exercised.
   5. **A single-owner fixture makes visibility predicates unobservable.** Seed 
two owners with distinct names and distinct creation times, and vary `isPublic` 
/ `isDownloadable` — otherwise spurious-visibility mutants survive.
   6. **There is a known production defect here: `stopAdaptiveBatching` cancels 
the handle without clearing it**, so after a real start-then-pause, 
`resumeAdaptiveBatching` cannot re-arm at all. Adding the clear (the 
`CoordinatorTimerService.disableTimer` behaviour) is a production change. Leave 
that behaviour unpinned in both directions rather than asserting current 
behaviour and cementing it.
   7. Note also that nothing in production reads `isPaused` — only the service 
itself — so any `isPaused` contract a spec pins is characterization, not 
specification.
   
   New spec files need the Apache license header or the license check fails.
   
   ### Task Type
   
   - [ ] Refactor / Cleanup
   - [ ] DevOps / Deployment / CI
   - [x] Testing / QA
   - [ ] Documentation
   - [ ] Performance
   - [ ] Other
   


-- 
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]

Reply via email to