Skip to content

Test bundled protocol schema through the production loader - #2772

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/protocol-schema-startup-tests-2708
Oct 15, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/protocol-schema-startup-tests-2708

Conversation

@kavemang

@kavemang kavemang commented Oct 11, 2026 •

Copy link
Copy Markdown

What this PR does

The protocol smoke test only checks WhoopProtocolInfo.schemaResourceURL, an accessor with no runtime callers. A URL can exist while the schema's dynamic-field decoders are disconnected from parseFrame.

Remove that unused accessor and test the real loadSchema startup path instead. The smoke test checks the bundled packet roster, resolves every packet through the dispatch index, and verifies that every declared post-hook is registered. This catches missing hook registration or a schema hook typo before fields silently disappear.

The removed API is specific to SwiftPM resource packaging and has no Kotlin twin. Runtime schema loading, decoding, wire bytes and schema data are unchanged.

Type of change

  • Refactor / cleanup
  • CI / tooling (regression tests)

How it was tested

  • Full Linux WhoopProtocol suite with noop-dev run-brief -- swift test --jobs 1: 816 tests, one skipped, zero failures.
  • Private isolated package mutation removing post-hook registration: clean compile succeeded, then the new smoke test failed all seven missing-hook assertions. No mutation is committed.
  • Guarded parity snapshot refresh passed (289 known findings); the diff removes one file, one function and one test-only finding without changing shared pair sets or dispositions. Core Tools: 153 tests passed. Source-comment hygiene, diff-scoped i18n and whitespace checks passed.
  • Native CI at unchanged head 86fcb79ef0e96ba4deeebed4531d3f58d91703d6: all 17 expected checks passed, with a stable complete roster confirmed twice.
  • Swift Packages CI: all eight package and three tool jobs passed. Native WhoopProtocol ran 816 tests, one skipped, zero failures; SmokeTests.testBundledSchemaLoadsAndWiresEveryPacketDecoder explicitly passed. Backfill has no test target and remains build-only.
  • Apple CI: universal macOS Strand build and iOS NOOPiOS build passed; macOS StrandTests ran 2,279 tests, two skipped, zero failures. iOS is compile-only; its test step was skipped by workflow design.
  • Parity Governance CI: 160 tests passed. Tools CI: 234 capture, 50 repository acceptance, 2 R-R preservation and 153 core tests passed. Hygiene and i18n also passed.
  • No warnings naming the changed test or removed source file in native protocol/macOS/iOS logs. Apple merge-current-main steps were skipped, so validation covers the actual PR head rather than a hypothetical merge with later main.
  • No strap/radio hardware test performed. No BLE commands, connection state or live transport behavior changes.

Checklist

  • Changed Swift package suite passes
  • Native package/app checks verified at PR head
  • No warnings naming changed files in native build logs
  • No UI styling or protocol literals added
  • Conventions followed; SwiftPM-only removal has no Kotlin counterpart
  • No generated build output, identifiers, private notes or secrets committed

Related issues

Refs #2708

@kavemang
kavemang marked this pull request as ready for review October 11, 2026 05:24
@ryanbr

ryanbr commented Oct 15, 2026

Copy link
Copy Markdown
Owner

Reviewed and verified locally. Green at 86fcb79, 17/17.

WhoopProtocolInfo had exactly one reference anywhere in the tree, the assertion this PR replaces, so the removal is clean and the Packages/WhoopProtocol test-only-callsite count going 30 to 29 matches it.

The new assertion is a genuine upgrade rather than a reshuffle. registerPostHooks() is called from inside loadSchema() at Schema.swift:230, so walking schema.packets and requiring postHooks[packet.post] to resolve exercises the registration side that a resource URL could never see. I checked the expected set against the shipped whoop_protocol.json: the seven packet names match exactly, and every packet carries a post hook, so the XCTUnwrap(packet.post) line pins something real rather than passing vacuously.

The alias loop is a no-op today since no packet declares aliases, but it costs nothing and is ready for the first one that does.

Ledger counts line up with a single unpaired Swift function leaving scope: files 510 to 509, functions 4538 to 4537, unpaired_files 370 to 369, unpaired_functions 4099 to 4098.

Good to merge.

@ryanbr
ryanbr merged commit 7a00c26 into ryanbr:main Oct 15, 2026
17 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