Skip to content

Test workout catalog icons through the production resolver - #2769

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/workout-icon-production-tests-2708
Oct 15, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
kavemang:agent/workout-icon-production-tests-2708

Conversation

@kavemang

@kavemang kavemang commented Oct 11, 2026 •

Copy link
Copy Markdown

What this PR does

The workout catalog uniqueness test currently reads a separate preferred-icon switch, so changing the glyph used by the UI can leave that test green. Use the production availability-aware identity resolver instead and remove the test-only switch. Keep the package runtime uniqueness guard and add symbol-bridge coverage for system glyphs, the custom Padel icon, and unknown/blank workouts.

No icon mappings, availability fallbacks, drawing, or stored sport labels change. This is Apple-specific SF Symbol test coverage; Android has no equivalent preferredIdentity helper, and no cross-platform data or analytics behavior changes.

Type of change

  • Refactor / cleanup

How it was tested

  • Linux targeted harness: all 18 committed tests pass (9 WorkoutTypeIconTests, 9 WorkoutCatalogTests), compiling the actual pure iconography/resolution/bridge code and workout catalog. The existing Linux availability branch assumes symbols exist; this is not native SwiftUI or Apple symbol-availability validation.
  • Private mutation giving Walking the Running glyph fails both runtime uniqueness guards, including the retargeted app catalog test. Mutation is not committed.
  • Core Tools: 153 tests pass. Source hygiene, diff-scoped i18n and whitespace pass.
  • Actual head 3fada881befa04e7b526ed2850977e439f47896c: all 16 expected CI checks passed, confirmed twice with a stable complete roster.
  • Native StrandDesign package: 135 tests, zero failures; all nine WorkoutTypeIconTests passed, including the two new symbol-bridge tests. All eight package and three tool matrix jobs passed; Backfill is build-only.
  • Universal macOS Strand build and StrandTests passed: 2,279 tests, two skipped, zero failures. All nine WorkoutCatalogTests passed, including the retargeted runtime uniqueness guard.
  • iOS NOOPiOS build passed. Its test step is skipped by workflow design, so this is compile-only evidence. App workflows skipped merging current main; validation is of the actual PR head, not a speculative later merge.
  • Tools CI: 234 capture, 50 parity repository-acceptance, two RR preservation, and 153 core tests passed. Hygiene and i18n passed. No warnings name the changed files in native package or app logs.

Checklist

  • Swift package tests pass for any package I touched (native StrandDesign CI passed)
  • Android unchanged
  • No new build warnings in changed files (native package and app logs reviewed)
  • No UI style changes
  • No hardcoded hex frame bytes
  • Follows docs/CONTRIBUTING.md
  • No generated output or secrets committed

Related issues

Refs #2708

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

ryanbr commented Oct 15, 2026

Copy link
Copy Markdown
Owner

Reviewed and verified locally. Green at 3fada88, 16/16.

I pulled both tables out of the pre-merge file and compared them case by case before agreeing with the deletion: preferredIdentity and the preferred argument of glyph(for:) agree on all 101 cases, so nothing is lost by routing the uniqueness check through identity(for:). The deleted copy was 101 hand-maintained lines with no test asserting it still matched the resolver it mirrored, which is exactly the shape worth deleting.

testSportSymbolBridgeUsesRuntimeSystemGlyphs is real new coverage: it ties sportSymbol to glyph(for:) for every case, so a catalogue name that resolves to the wrong type now shows up. The custom and unknown cases pin padelRacket to figure.tennis and the empty string to figure.mixed.cardio, neither of which testSportSymbolBridgeNonEmpty could have caught.

One thing for a follow-up rather than for this PR. Every remaining uniqueness test runs through glyph(for:), which resolves availability at runtime, so they only ever see the preferred symbol on a runner where every symbol exists. Once a fallback engages, 16 glyph names become reachable by two or more types, and figure.martial.arts by four:

  • figure.martial.arts: jiuJitsu, judo, muayThai, martialArts
  • figure.dance: ballet, breakdancing, dancing
  • figure.walk: walking, nordicWalking
  • bicycle: cycling, mountainBiking
  • figure.hockey: hurlingCamogie, iceHockey

plus eleven more pairs. The comment at SportIcon.swift:230 promises each sport a glyph no other sport uses and says each carries a fallback for an OS without the preferred symbol, and the fallbacks are where that promise is not kept. A pure test over the fallback arguments, with no availability call in it, would reach the case no CI runner can.

Nothing blocking. Good to merge.

@ryanbr
ryanbr merged commit 8801d95 into ryanbr:main Oct 15, 2026
16 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