Repository navigation
build: restore the Apple targets, and the app-target tests that never compiled - #1892
Merged
Merged
Conversation
…them `DailySkinTempAbsoluteCarryTests` has not compiled since #1879. `DailyMetric`'s initializer defaults only from `spo2Pct` onward — its first twelve parameters are required — and the sleep-carry cases I added there call compact forms like `DailyMetric(day:sleepHrOnly:)` that do not exist. Swift resolves those against `takingSleepFields(from:)` instead, which is why the error reads "missing argument for parameter 'from'" and points nowhere useful. The file already had a `row(...)` helper filling the required twelve; the new cases simply did not use it. It now also carries the sleep fields they need, and every case goes through it, so none can drift back into a compact call. This shipped to main and stayed there because `StrandTests` runs only in the on-demand app-build, never in default CI, so a red app target is invisible until someone dispatches it. It surfaced now only because #1862 needed the Swift compile gate for its own reasons.
`WhoopSerialIdentity.logSafe(serial:)` takes a labelled argument and the call added in #1635's copy change passed it positionally. Kotlin accepts that, so the Android build stayed green and the twin looked fine; Swift does not, and both Apple targets have failed to compile since that merge. It reached main because a PR touching only `Strand/` and `android/` runs no check that compiles Swift: the package matrix needs `Packages/`, and StrandTests only runs in the on-demand app-build. A green tick on that PR meant Android and the Python tools, nothing more.
Two more the compile gate surfaced once the first layer cleared. `DailyMetric.fillingNilFields` and `takingSleepFields` sit in a `private extension`, which even `@testable import` cannot see, so the cases exercising them could never have compiled. Widened to internal, still module-confined. They stayed private because nothing demanded otherwise: the tests calling them have never run. `DayCycleRecoveryTests` referenced `UserProfile` without importing StrandAnalytics, which arrived with the day-cycle work in #1572 and has been red since. Both are the same story as the rest of this branch. StrandTests runs only in the on-demand app-build, so a test file can be merged in a state where it does not compile and stay that way indefinitely.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
maindoes not currently compile on iOS or macOS. This restores both, andapp-buildis green on this branch (run 33952090013) after four rounds of the gate reporting one layer at a time.What was broken
WhoopSerialIdentity.logSafe(serial:)called positionally inFrameRouterDailyMetric(day:sleepHrOnly:)compact inits — the initializer's first twelve parameters are requiredfillingNilFields/takingSleepFieldsin aprivate extension, invisible even to@testableDayCycleRecoveryTestsusingUserProfilewithout importing StrandAnalyticsThe
logSafeone is the live breakage: Kotlin accepts a positional argument so the Android twin compiled and looked correct, while Swift requires the label and both Apple targets stopped building.Why none of it was caught
A PR touching only
Strand/andandroid/runs no check that compiles Swift. The package matrix needsPackages/;StrandTestsruns only in the on-demandapp-build, which is disabled by default. So a green tick on such a PR means Android and the Python tools — nothing about the Apple targets.That is how four independent breakages accumulated unnoticed, one of them from a community PR. It also means a test file can be merged in a state where it has never compiled and stay that way indefinitely, which is what happened to two of the four above.
The fixes
Each is the smallest thing that makes the code reachable, not a rewrite:
DailySkinTempAbsoluteCarryTestsalready had arow(...)helper filling the twelve required parameters; the sleep-carry cases now use it and it carries the fields they need, so none can drift back into a compact call.private extensionbecomesinternal— still module-confined. It stayed private only because the tests calling it never ran.Verification
app-buildSUCCESS on both targets — the only check that compiles this code.Worth doing separately
This will recur. Either make
app-buildrequired forStrand/**changes, or add a cheap compile-only job to the default checks. Without one, the next Swift-only PR merges with the same blind spot.