Skip to content

Prune redundant tests and the dead code they covered - #346

Merged
jlipworth merged 9 commits into
mainfrom
chore/test-suite-cleanup
Oct 16, 2026
Merged

jlipworth merged 9 commits into
mainfrom
chore/test-suite-cleanup

Conversation

@jlipworth

Copy link
Copy Markdown
Owner

Summary

A test-suite audit across all ~390 test files (PMSKit, LabstreamTests, the Mobile/TV UI tests, and scripts/tests). Each of 8 domain clusters went through the same steps: an audit pass proposed changes, an adversarial reviewer checked each one, and only the changes it accepted were applied. Every removal or merge had to name the surviving test that still covers the behaviour, or show (with rg) that the code under test is dead.

  • Test functions: 1680 → 1393 (−287); 182 changes applied, 4 rejected by the reviewer.
  • What was removed: tests of unreferenced code; tests of Swift-synthesized Equatable/Codable; exact duplicates (including tests that were the union of two neighbouring tests); forwarding aliases tested twice.
  • What was merged: near-identical cases folded into parameterized tables; split files for the same type merged.
  • What was kept: guards for the playback-architecture and hard-constraint invariants, regression tests tied to past bugs, and distinct edge cases.
  • One commit per cluster, plus a final commit that removes the production code those tests were the only users of:
    • PMSKit LibraryGridLayout
    • the legacy OptimizeRequest.Target / create / statusRequest (PUT /library/optimize) path
    • the removableItemIDs and selectedMediaSourceID aliases
    • the tvOS keyboard / keyboard-shell TVUI-004 bisection fixtures. The live Search-tab keyboard test supersedes them, and the matching release-binary marker is dropped from the performance audit contract.

Validation (local)

  • swift test --package-path PMSKit --no-parallel --skip 'Live.*ProbeTests': 1520 Swift Testing + 109 XCTest, 0 failures
  • uv run python -m unittest discover -s scripts/tests: 358 OK. Needs /usr/bin first on PATH locally because of a broken Homebrew swiftc; unrelated to this change.
  • native-test-matrix.py affected lanes:
    • builds: visionOS, mobile, tvOS (build-for-testing), macOS — all succeeded
    • hosted tests: macOS passed; mobile 616 + 80 XCTest, 0 failures; tvOS 364 + 52 XCTest, 0 failures
    • UI smoke: mobile and tvOS both passed
  • visionOS clean build → install → launch smoke on the worktree sim: the installed UUID matches the build and the process stays alive with no app-level errors. It lands on Plex sign-in because the worktree sim clone carries no session; this branch does not touch auth.

Open gates

  • The full tvOS player UI-test suite was not run. Only the smoke lane ran; it is the run that would exercise the folded testHiddenChromeEvidenceSweep / testBasicPlaybackRevealAndExit.
  • TVUI-004 physical Siri Remote acceptance remains open (unchanged).
  • Live *ProbeTests were not run (opt-in, need real servers). Probe files were only trimmed where their subject was covered or dead.

Removes tests that duplicated stronger coverage or pinned dead code, and
folds repetitive cases into parameterized tables (269 -> 226 test funcs).

- Delete OptimizeTests: OptimizeRequest.create/Target/statusRequest tests
  covered code with no non-test callers; the two meaningful downloadURL
  encoding tests move into OptimizePlaylistTests.
- Merge JellyfinKeepaliveHealthPolicyTests into
  JellyfinDownloadKeepalivePolicyTests.
- Drop single-field round-trip / tautological tests (removableItemIDs,
  selectedMediaSourceID, compoundKey, encodeProducesVersionedEnvelope,
  ratingKeyPrefix expectations, etc.); staleItemIDs now covers the
  completed/successful statuses the removed tests exercised.
- Parameterize existing-version playability, background range/opaque
  completion, finalization outcome, Jellyfin route semantics and transcode
  speed decoding, keeping every case and its rationale comments.
- Move the dimensions parser test into DownloadResolutionLabelTests and
  the terminal-auth-message suite into PostLogoutDownloadFailurePolicyTests.
Removes overlapping LabstreamTests download-store, cleanup-journal,
keepalive, work-registry and options-model tests whose behavior is pinned
by stronger neighbouring tests, and consolidates near-duplicate cases
(196 -> 166 test funcs across 14 files).
Removes duplicate request-shape and policy assertions across PMSKit
playback, transcode, DV/HDR, music and cinema-exit suites and the app
playback lifecycle/track-selection tests, folding repetitive cases into
parameterized tables (253 -> 187 test funcs).

tvOS UI: drops the four system-keyboard bisection fixtures tests (the live
Search-tab keyboard test covers every layer they replicated; the
keyboard/keyboard-shell fixtures are now unused and left for a follow-up)
and the directional/select reveal and back-exit tests already covered by
testHiddenChromeEvidenceSweep and testBasicPlaybackRevealAndExit.

The scrobble/unscrobble table uses path strings rather than static-method
references to avoid Sendable inference issues in Swift Testing arguments.
- Delete EmbyServerURLTests/JellyfinServerURLTests; normalization is
  pinned by the shared golden table in MediaBrowserIdentityCompatibilityTests
  (c5 commit carries the /emby base-path divergence comment).
- Drop duplicated auth-result decoding, header, user-views, items-request
  and X-Application tests already covered by wire goldens.
- Parameterize apiBase normalization, PIN request strength, keychain
  migration policy (exhaustive 8-row table), terminal/success flags and
  credential artifact writers; merge recovery/media session config tests.
- Use the shared queryMap helper and TestFixtures in Jellyfin/Emby library
  tests.

167 -> 124 test funcs.
Deletes LiveBIFIndexProbeTests and trims redundant live-probe config and
static-range (finished body, reattach, segment marker, continuation,
remainder request, train integrity, resume data, segment assembly) cases
whose behavior is covered by the remaining tables (95 -> 68 test funcs).
Deletes LibraryGridLayoutTests (the layout type has no app callers; source
removal is left as a follow-up) and removes duplicate decoding, browse
query, alphabet bucket, version-collapse, home rails, TV hierarchy and
misc request tests, consolidating the rest into tables (262 -> 219 test
funcs). Adds the /emby base-path divergence note to the shared backend URL
golden table now that the per-backend URL tests are gone.
Deletes BrowseLoadStateTests and removes overlapping artwork, catalog,
paging, metadata repository, persistence writer, preference store and
test-support tests whose behavior is pinned elsewhere (204 -> 187 test
funcs).
Removes duplicated or tautological cases across the scripts/tests tooling
suites (platform runners, screenshots, perf, privacy manifest, source
topology, TestFlight release, tooling hardening) and consolidates the
remainder (232 -> 216 test functions).
The test-suite prune removed tests whose only subject was unreferenced
production code. Delete that code too:

- PMSKit LibraryGridLayout (no callers; LibraryGridView builds its
  GridItem directly) and the stale comment pointing at its tests.
- OptimizeRequest.Target / create(...) / statusRequest(...): the legacy
  flat PUT /library/optimize path. The app only uses the type-42
  background-processing playlist path (createOnPlaylist).
- BackgroundProcessingItems.removableItemIDs and
  DownloadExistingVersionOptionPolicy.selectedMediaSourceID: one-line
  aliases with no callers.
- tvOS keyboard / keyboard-shell UI-test fixtures (TVUI-004 bisection).
  Their UI tests were superseded by the live Search-tab keyboard test;
  the physical-remote gate for TVUI-004 stays open. Drop the matching
  TVKeyboardFixture: release-binary marker from the performance audit
  contract (the --ui-testing-fixture marker still guards fixtures).
@jlipworth
jlipworth merged commit f278004 into main Oct 16, 2026
4 checks passed
@jlipworth
jlipworth deleted the chore/test-suite-cleanup branch October 16, 2026 01:57
jlipworth added a commit that referenced this pull request Oct 16, 2026
## Summary

Makes outside pull requests cheap to review by flagging the ones that
touch essential surfaces, and says plainly that a PR may be
re-implemented rather than merged.

- `.github/triage/sensitive-paths.txt` is the single list of sensitive
paths, grouped by category: signing and entitlements, auth and
credentials, the transcode profile, dependencies, CI and release
tooling, App Store text, agent instructions (`CLAUDE.md`, `AGENTS.md`,
`.claude/`, `.codex/`), and policies.
- `scripts/check-sensitive-paths.py` lists the matching files for a PR
(`--pr N`), a branch (`--base REF`), or a list of paths. For PRs it also
checks the old path of renamed files. It is read-only and never checks
out PR code.
- The new `sensitive-paths` label has been created. Under BOT-POLICY the
bot applies it and names the matched categories, without judging whether
the change is safe. It never removes the label.
- `docs/CONTRIBUTING.md` and the PR template: PRs are proposals. The
maintainer may merge one or re-implement it with credit, and
sensitive-path changes usually get re-implemented.
- `CLAUDE.md`: before proposing to merge someone else's PR, run the
checker, report every match, and review those files line by line.

GitHub Actions is disabled for this repo, and Woodpecker has no token
that can label PRs, so the label is applied by the bot (or by hand), not
by CI.

## Linked issue

None. This is a follow-up to #348.

## Affected surface

- Platforms: none (tooling and docs)
- Backends: none

## Verification evidence

- [x] Commands run and results:
  - 5 new unit tests pass.
- `scripts/ci-hygiene.sh` passes locally: strict MkDocs build, link and
Mermaid checks, tooling tests.
- Checker on recent PRs: #346 finds nothing; #342 finds the TestFlight
script and both skill mirrors; this branch finds 5 files.
- [x] Simulator / host / device evidence: not applicable, no app code.
- [ ] Not verified (and why): the bot behavior. The bot runs on the
maintainer's own accounts.

## Privacy checklist

- [x] No tokens, client identifiers, passwords, or signing/team
identifiers.
- [x] No real server hostnames, IPs, or URLs.
- [x] No usernames, media titles, filenames, library paths, or personal
home-directory paths.
- [x] Security-sensitive changes were discussed privately first (n/a).

## AI assistance

Claude Opus via Claude Code wrote the change. The maintainer reviews it.
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.

1 participant