Skip to content

test: cover WHOOP 5 gap regressions on both platforms - #2056

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
Trillient:test/whoop5-gap-regressions
Sep 10, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
Trillient:test/whoop5-gap-regressions

Conversation

@Trillient

@Trillient Trillient commented Sep 10, 2026 •

Copy link
Copy Markdown

What this PR does

Add regression coverage for the WHOOP 5 missing-score explanation introduced in dfb69e67. The Swift and Kotlin tests run nightly scoring against stored data, then feed the persisted daily row and the real first-recorded/first-scorable queries into the same Whoop5RR.legacyUnscorableNight predicate used by Today.

The final diff changes two test files. Production code, UI copy, and the historical date bounds match main.

Type of change

  • Bug fix regression coverage
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

How it was tested

The new lifecycle coverage pins three cases:

  1. A staged WHOOP 5 night containing only unlabelled beats has neither HRV nor Charge despite a seeded baseline, and qualifies for the explanation.
  2. Re-offloading identical interval keys with canonical source labels inserts zero rows, restores HRV and Charge, and clears the explanation through the next cached scoring pass.
  3. WHOOP 4 legacy beats remain scoreable. Ordinary missing beats and valid HRV with missing Charge do not qualify for the explanation. The calibration control retains the positive legacy case's date bounds and changes only displayed HRV, so it cannot pass merely because the first-scorable date suppresses the note.

Existing re-pairing/alias, transport-window and restaging regressions remain intact. These are scorer/store/predicate tests; they do not render the UI.

  • macOS app build and all 5 IntelligenceRRSourceTests passed locally. The Apple CI run also passed its full macOS test run and iOS simulator build.
  • All 27 focused Android R-R, SQLite and JaCoCo-budget tests passed. The full Android CI run also passed.
  • Removing the HRV guard deliberately made the new WHOOP 5 test fail on both platforms; the original production files were restored byte-for-byte before the final passing runs.
  • Full Android local suite: 5,847 tests, 6 skipped, with only the two previously reproduced exact half-tie failures in unchanged RecoveryDriversTest on JetBrains JDK 21.0.8 (-0.5 versus -0.4999999999999929).
  • Source-comment lint and whitespace checks passed.

Checklist

  • Swift app tests pass locally and the full macOS test job passes in CI
  • Focused Android tests pass
  • Full Android suite passes in CI — the local JVM-specific rounding assertions are described above
  • No production, UI, protocol or schema changes
  • Follows docs/CONTRIBUTING.md
  • No generated Xcode project, credentials, or private captures committed

Related issues

Follow-up to #2046 and the gap explanation in dfb69e67. Related to #1505.

@ryanbr ryanbr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at f4fcbb8. This is the layer my own tests for the explanation did not reach, and it is the
right one to add. Approving.

What it covers that the existing tests do not. The predicate and the two store reads were each
pinned in isolation. This drives the whole path: analyzeRecent into the persisted row, the row plus
the production queries into the predicate, and then the same night again. That is where a regression
would actually live, and the lifecycle it walks is the one the wearer walks. Beats absent, so no claim.
Legacy beats present, so the claim stands. A re-offload arriving labelled, so the promotion lands, the
night scores, and the claim goes. Then a re-score on top, which is the case I would have missed: a
cache hit must not revive an explanation for a night that now has a Charge.

Both false-positive directions are pinned, which for a diagnostic matters more than the positive
one.
A night with no beats at all is not blamed on the units, and a night with valid HRV but no
Charge stays ordinary calibration rather than being handed a cause it does not have. The WHOOP 4 case
asserts the whole mechanism stays off a family it was never about. Those three are what stop the
explanation growing into a catch-all for an empty ring.

insert(...).rr == 0 on the labelled re-offload is a good detail. It pins that a re-offload
changes provenance rather than adding a second copy of the night, which is the part of the promotion
path that would be invisible in a test that only checked the score came back.

Verified rather than taken on trust: I merged this onto current main, which is two commits ahead of
its base, and ran the Android class. Eighteen tests, no failures, and both new cases present in the
XML rather than filtered to nothing. All five checks are green on the PR's own head.

Two notes, neither blocking:

  1. showsLegacyGap rebuilds the view's assembly rather than calling it, so what is pinned is the
    predicate and the queries, not the wiring. The owner it passes is given to it; the shipped code
    resolves that itself, and given how much re-pair alias history this area carries, the owner
    resolution is the part I would least like to be uncovered. Both platforms would stay green if a
    view started asking about the wrong strap. Extracting the assembly into one function the view and
    the test both call would close it, the way the hosted trend cards ended up sharing their resolver
    rather than each having one.
  2. The Swift seedBaseline walks back by subtracting 86,400 seconds and reformatting, where the Kotlin
    twin uses LocalDate.minusDays. Across a DST boundary the arithmetic version can repeat or skip a
    calendar date, which here costs one of eight seeded days and so cannot fail the test. Worth knowing
    it is the same shape that did bite a day-key helper recently, and the calendar call is no longer.

Thanks @Trillient. Tests that walk the lifecycle rather than the unit are the harder ones to write and
the ones that survive a refactor.

@ryanbr
ryanbr merged commit f23fd54 into ryanbr:main Sep 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants