test+refactor: move Entity logic, add coverage tests — +26 tests#174
Conversation
LRC format parsing, plain/synced priority, malformed line handling, nil/empty fallback, whitespace trimming. Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRefactors lyrics parsing from the entity into the use-case, moves FetchState convenience accessors to the presenters layer, updates call sites to use the new parser method, adds parsing/config/sqlite tests, and updates the TrackHandlerTests test target dependency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
FetchState: value, isLoading, isRevealing, isIdle for all states. LyricsContent: LRC parsing, synced/plain priority, malformed handling. Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a specification-style test suite to lock in the current LyricsContent parsing behavior (plain vs synced/LRC), covering several edge cases and fallback rules.
Changes:
- Added
LyricsContent.init(from: LyricsResult?)tests for nil/no-lyrics, plain splitting, and synced-priority behavior. - Added LRC parsing tests for standard format, multiple lines, integer seconds, malformed line skipping, fallback-to-plain, and whitespace trimming.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codable round-trip, default init, missing optional decoding. Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/EntityTests/LyricsContentTests.swift (1)
50-60: Consider floating-point tolerance for timestamp comparisons.Direct
==comparisons with floating-point values can be fragile due to IEEE 754 representation. While these specific values work, using an approximate comparison would be more robust for future maintenance.💡 Optional: Use approximate equality
Swift Testing doesn't have built-in approximate equality, but you could use a small helper or inline check:
- `#expect`(lines[0].time == 83.45) + `#expect`(abs(lines[0].time - 83.45) < 0.001)This is optional since the current tests pass and the specific values used are well-behaved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/EntityTests/LyricsContentTests.swift` around lines 50 - 60, The test standardLRC uses a direct equality check for the parsed timestamp (lines[0].time == 83.45) which is fragile for floating-point values; change the assertion to use an approximate comparison with a small tolerance (e.g., check abs(lines[0].time - 83.45) < epsilon or use XCTAssertEqual with an accuracy parameter) when validating LyricsContent(from: LyricsResult(...)) in the standardLRC test so the comparison is robust to minor floating-point rounding; keep the other expectations (lines.count and lines[0].text) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Tests/EntityTests/LyricsContentTests.swift`:
- Around line 50-60: The test standardLRC uses a direct equality check for the
parsed timestamp (lines[0].time == 83.45) which is fragile for floating-point
values; change the assertion to use an approximate comparison with a small
tolerance (e.g., check abs(lines[0].time - 83.45) < epsilon or use
XCTAssertEqual with an accuracy parameter) when validating LyricsContent(from:
LyricsResult(...)) in the standardLRC test so the comparison is robust to minor
floating-point rounding; keep the other expectations (lines.count and
lines[0].text) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc18c070-4a7d-4f2c-b4f5-be00af2e16ea
📒 Files selected for processing (1)
Tests/EntityTests/LyricsContentTests.swift
GeneralD
changed the title
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- LyricsContent.init(from:)/parseSyncedLyrics → LyricsUseCase.parseLyricsContent - FetchState computed properties → Presenters/FetchState+Convenience.swift - Entity is now pure data types only - Tests moved from EntityTests to LyricsUseCaseTests - Entity tests for FetchState/NowPlayingInfo removed (pure data, no logic) Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GeneralD
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
sanitizeTomlFloats: truncation, short decimals, strings, integers. tryDecode: no file, valid TOML, invalid TOML. Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/LyricsUseCase/LyricsUseCaseImpl.swift`:
- Around line 25-27: The splitting and trimming for plain lyrics currently only
uses "\n" and .whitespaces, which lets CR (\r) remain in lines; update the
processing of result.plainLyrics (the code that returns
.plain(plain.components(separatedBy: "\n"))) to split using
CharacterSet.newlines (or .components(separatedBy: .newlines)) and change any
trimming calls that use trimmingCharacters(in: .whitespaces) to
trimmingCharacters(in: .whitespacesAndNewlines) so CRLF input is normalized and
carriage returns are removed from lyric lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f706223-de06-48f9-ad0b-2fcc8bef4520
📒 Files selected for processing (11)
Package.swiftSources/Domain/Dependencies/UseCase/LyricsUseCase.swiftSources/Entity/FetchState.swiftSources/Entity/LyricsContent.swiftSources/LyricsUseCase/LyricsUseCaseImpl.swiftSources/Presenters/FetchState+Convenience.swiftSources/TrackHandler/TrackHandlerImpl.swiftSources/TrackInteractor/TrackInteractorImpl.swiftTests/LyricsUseCaseTests/LyricsContentParsingTests.swiftTests/TrackHandlerTests/TrackHandlerImplTests.swiftTests/TrackInteractorTests/TrackInteractorRaceTests.swift
💤 Files with no reviewable changes (2)
- Sources/Entity/LyricsContent.swift
- Sources/Entity/FetchState.swift
✅ Files skipped from review due to trivial changes (1)
- Tests/TrackInteractorTests/TrackInteractorRaceTests.swift
Table creation, unique constraints, primary key, idempotency. Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GeneralD
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Tests/ConfigDataSourceTests/TryDecodeTests.swift (1)
21-24: Extract env/temp setup into a helper to reduce duplication and prevent drift.
validTomlandinvalidTomlrepeat nearly identical temp-dir + env wiring. A small helper would make cleanup/restoration consistent across tests.♻️ Refactor sketch
+private func withIsolatedConfigHome(_ body: (_ tmp: String) throws -> Void) throws { + let tmpURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: tmpURL, withIntermediateDirectories: true) + let prevXdg = ProcessInfo.processInfo.environment["XDG_CONFIG_HOME"] + defer { + if let prevXdg { setenv("XDG_CONFIG_HOME", prevXdg, 1) } else { unsetenv("XDG_CONFIG_HOME") } + try? FileManager.default.removeItem(at: tmpURL) + } + setenv("XDG_CONFIG_HOME", tmpURL.path, 1) + try body(tmpURL.path) +}Also applies to: 29-31, 39-42, 46-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/ConfigDataSourceTests/TryDecodeTests.swift` around lines 21 - 24, Extract the repeated temp-dir and env wiring into a small test helper (e.g., withTempLyraEnv or makeTempLyraDirectory) that creates the temporary directory and "lyra" subdir, sets the environment variables used by the tests, returns the tmp path/lyraDir (or executes a closure), and ensures cleanup and environment restoration via defer; replace the duplicated blocks that set tmp, lyraDir, createDirectory and defer removal in TryDecodeTests.swift (the spots that set up validToml and invalidToml) with calls to this helper so cleanup/restoration logic is centralized and consistent across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Tests/ConfigDataSourceTests/TryDecodeTests.swift`:
- Around line 10-12: The test `noFile` in TryDecodeTests.swift fails to fully
isolate config lookup because only XDG_CONFIG_HOME is overridden; update the
test to also override HOME to a temp directory (created via
FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path)
so findConfigFile() cannot see an existing ~/.lyra, and save any existing
environment values for "XDG_CONFIG_HOME" and "HOME" before calling setenv; then
restore the original values in defer (instead of blindly
unsetenv("XDG_CONFIG_HOME")), ensuring both env vars are restored to their
previous states after the test completes.
In `@Tests/SQLiteDataStoreTests/DatabaseManagerMigrationTests.swift`:
- Around line 82-86: The test "idempotent" is invalid because using
DatabaseManager(inMemory: true) creates separate in-memory databases; instead
run migrations twice against the same database instance or queue: instantiate
one DatabaseManager, then call its migrate() (or expose and call a new migrate()
that runs DatabaseMigrator) twice, or reuse its dbQueue to perform the migrator
twice; update the test to either (a) call DatabaseManager.migrate() two times on
the same DatabaseManager instance or (b) create a persistent (non-inMemory)
DatabaseManager and run migrations twice on that single instance to assert no
error.
---
Nitpick comments:
In `@Tests/ConfigDataSourceTests/TryDecodeTests.swift`:
- Around line 21-24: Extract the repeated temp-dir and env wiring into a small
test helper (e.g., withTempLyraEnv or makeTempLyraDirectory) that creates the
temporary directory and "lyra" subdir, sets the environment variables used by
the tests, returns the tmp path/lyraDir (or executes a closure), and ensures
cleanup and environment restoration via defer; replace the duplicated blocks
that set tmp, lyraDir, createDirectory and defer removal in TryDecodeTests.swift
(the spots that set up validToml and invalidToml) with calls to this helper so
cleanup/restoration logic is centralized and consistent across tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c7bac98-337f-4e05-a710-f0cfb2eee9ac
📒 Files selected for processing (3)
Tests/ConfigDataSourceTests/SanitizeTomlFloatsTests.swiftTests/ConfigDataSourceTests/TryDecodeTests.swiftTests/SQLiteDataStoreTests/DatabaseManagerMigrationTests.swift
| let tmp = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path | ||
| setenv("XDG_CONFIG_HOME", tmp, 1) | ||
| defer { unsetenv("XDG_CONFIG_HOME") } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "TryDecodeTests.swift" --exactRepository: GeneralD/lyra
Length of output: 281
🏁 Script executed:
fd -t f "ConfigDataSourceImpl.swift" --exactRepository: GeneralD/lyra
Length of output: 281
🏁 Script executed:
fd "TryDecodeTests.swift"Repository: GeneralD/lyra
Length of output: 105
🏁 Script executed:
fd "ConfigDataSourceImpl.swift"Repository: GeneralD/lyra
Length of output: 108
🏁 Script executed:
cat -n "Tests/ConfigDataSourceTests/TryDecodeTests.swift"Repository: GeneralD/lyra
Length of output: 2375
🏁 Script executed:
cat -n "Sources/ConfigDataSource/ConfigDataSourceImpl.swift" | head -100Repository: GeneralD/lyra
Length of output: 4457
🏁 Script executed:
# Verify the search order in findConfigFile - confirm all candidate paths
cat -n "Sources/ConfigDataSource/ConfigDataSourceImpl.swift" | sed -n '67,88p'Repository: GeneralD/lyra
Length of output: 1115
🏁 Script executed:
# Check if Folder.home respects HOME environment variable
rg "Folder.home" --type swift -A 2 -B 2Repository: GeneralD/lyra
Length of output: 4313
noFile test lacks isolation and can fail on machines with existing ~/.lyra config.
findConfigFile() searches both XDG_CONFIG_HOME/lyra and HOME/.lyra, but the test only isolates XDG_CONFIG_HOME. If a developer has an existing ~/.lyra/config.toml on their machine, this test will find it and return a non-empty path instead of an empty string, causing a false failure. Additionally, using unsetenv("XDG_CONFIG_HOME") does not restore a previously set value, leaking side effects to other tests.
Suggested fix
`@Test`("returns empty string when no config file exists")
func noFile() throws {
- let tmp = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path
- setenv("XDG_CONFIG_HOME", tmp, 1)
- defer { unsetenv("XDG_CONFIG_HOME") }
+ let tmpURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+ try FileManager.default.createDirectory(at: tmpURL, withIntermediateDirectories: true)
+ let tmp = tmpURL.path
+ let prevXdg = ProcessInfo.processInfo.environment["XDG_CONFIG_HOME"]
+ let prevHome = ProcessInfo.processInfo.environment["HOME"]
+ setenv("XDG_CONFIG_HOME", tmp, 1)
+ setenv("HOME", tmp, 1)
+ defer {
+ if let prevXdg { setenv("XDG_CONFIG_HOME", prevXdg, 1) } else { unsetenv("XDG_CONFIG_HOME") }
+ if let prevHome { setenv("HOME", prevHome, 1) } else { unsetenv("HOME") }
+ try? FileManager.default.removeItem(at: tmpURL)
+ }
let ds = ConfigDataSourceImpl()
let result = try ds.tryDecode()
`#expect`(result == "")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let tmp = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path | |
| setenv("XDG_CONFIG_HOME", tmp, 1) | |
| defer { unsetenv("XDG_CONFIG_HOME") } | |
| let tmpURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) | |
| try FileManager.default.createDirectory(at: tmpURL, withIntermediateDirectories: true) | |
| let tmp = tmpURL.path | |
| let prevXdg = ProcessInfo.processInfo.environment["XDG_CONFIG_HOME"] | |
| let prevHome = ProcessInfo.processInfo.environment["HOME"] | |
| setenv("XDG_CONFIG_HOME", tmp, 1) | |
| setenv("HOME", tmp, 1) | |
| defer { | |
| if let prevXdg { setenv("XDG_CONFIG_HOME", prevXdg, 1) } else { unsetenv("XDG_CONFIG_HOME") } | |
| if let prevHome { setenv("HOME", prevHome, 1) } else { unsetenv("HOME") } | |
| try? FileManager.default.removeItem(at: tmpURL) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/ConfigDataSourceTests/TryDecodeTests.swift` around lines 10 - 12, The
test `noFile` in TryDecodeTests.swift fails to fully isolate config lookup
because only XDG_CONFIG_HOME is overridden; update the test to also override
HOME to a temp directory (created via
FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).path)
so findConfigFile() cannot see an existing ~/.lyra, and save any existing
environment values for "XDG_CONFIG_HOME" and "HOME" before calling setenv; then
restore the original values in defer (instead of blindly
unsetenv("XDG_CONFIG_HOME")), ensuring both env vars are restored to their
previous states after the test completes.
There was a problem hiding this comment.
Already has .serialized. The env var is set/unset with defer in each test.
There was a problem hiding this comment.
@GeneralD, you're right — .serialized combined with defer fully takes care of the env-var leak between tests. That part of my concern was mistaken.
However, the core isolation issue remains: findConfigFile() searches two paths — XDG_CONFIG_HOME/lyra and HOME/.lyra. The test only overrides XDG_CONFIG_HOME; HOME still points to the real home directory. If a developer has a real ~/.lyra/config.toml on their machine, findConfigFile() will find it via the HOME path and return a non-empty string, causing the noFile test to fail. Overriding HOME (or setenv("HOME", tmp, 1)) in addition to XDG_CONFIG_HOME would make the test fully hermetic regardless of the developer's local setup.
🧠 Learnings used
Learnt from: GeneralD
Repo: GeneralD/lyra PR: 145
File: Sources/WallpaperDataSource/FindExecutable.swift:3-13
Timestamp: 2026-03-28T00:56:56.181Z
Learning: In `Sources/WallpaperDataSource/FindExecutable.swift` (repository: GeneralD/lyra), `findExecutableInPath(_:)` is only called internally with hardcoded string literals (`"ffmpeg"`, `"yt-dlp"`, `"uvx"`). Empty or path-containing names are not a realistic concern; avoid flagging defensive guards for this function.
Learnt from: GeneralD
Repo: GeneralD/lyra PR: 153
File: Sources/AsyncParsableCommand/AsyncRunnableCommand.swift:29-43
Timestamp: 2026-04-04T15:35:39.936Z
Learning: In `Sources/AsyncParsableCommand/AsyncRunnableCommand.swift` (GeneralD/lyra), the `mutating run() throws` bridge intentionally does NOT write the boxed `command.wrappedValue` back to `self` after `semaphore.wait()`. All conforming commands (`TrackCommand`, `HealthcheckCommand`) communicate results via stdout and `ExitCode`, not via struct mutation. Do not flag the missing `self = command.wrappedValue` assignment as a bug; side effects are verified via an actor witness in tests, not by inspecting mutated struct fields.
Learnt from: GeneralD
Repo: GeneralD/lyra PR: 149
File: Sources/CLI/Commands/TrackCommand.swift:54-59
Timestamp: 2026-04-01T14:09:10.078Z
Learning: In `Sources/CLI/Commands/TrackCommand.swift` (GeneralD/lyra), the `resolvedMetadata(track:)` helper intentionally runs `metadataUseCase.resolveCandidates(track:)` when either `--resolve` OR `--lyrics` is set. This is because `resolveCandidates` strips noise words and brackets (e.g. `"【MV】Song Name / Artist - Official Video"`) and generates title/artist permutations that LRCLIB requires for accurate lyrics matching. Metadata resolution is a prerequisite for effective lyrics lookup, not an independent concern. Do not flag `guard resolve || lyrics` as incorrect coupling.
Learnt from: GeneralD
Repo: GeneralD/lyra PR: 145
File: Sources/WallpaperDataSource/FindExecutable.swift:3-13
Timestamp: 2026-03-28T00:56:53.421Z
Learning: In the Lyra Swift codebase, `ObjCBool` is intentionally avoided. During code review, do not recommend or introduce APIs that require an `ObjCBool` out-parameter; instead, prefer pure-Swift alternatives and Swift-first APIs. For example, when checking file executability, avoid suggesting `FileManager.fileExists(atPath:isDirectory:)` (which uses an `ObjCBool`) as a replacement for `isExecutableFile(atPath:)`.
Wallpaper configured with URL, configDir nil fallback. Part of #172 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GeneralD
changed the title
- parseSyncedLyrics uses .whitespacesAndNewlines to strip \r - Idempotency test uses file-based DB to verify same-DB migration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code Review ✅ Approved 2 resolved / 2 findingsRefactors Entity logic into LyricsContent and FetchState with 26 new comprehensive tests covering parsing, round-trip serialization, and migration scenarios. Fixed Windows line-ending handling in LRC parsing and idempotency test isolation; no issues remain. ✅ 2 resolved✅ Edge Case: parseSyncedLyrics keeps \r from Windows-style line endings
✅ Bug: Idempotency test uses separate in-memory DBs, not same DB
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Important Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/TrackHandlerTests/TrackHandlerImplTests.swift (1)
176-183: Delegation to real implementation is appropriate for this pure function.Since
parseLyricsContent(from:)is a pure, deterministic string parser with no I/O or side effects (as noted in the context snippet), delegating toLyricsUseCaseImplrather than stubbing avoids duplicating parsing logic and ensures realistic test behavior. This design choice is sound.Optional: Consider caching the
LyricsUseCaseImplinstance to avoid repeated allocation, though the impact is negligible for test code.,
♻️ Optional: Cache the impl instance
private struct StubLyricsUseCase: LyricsUseCase { let handler: `@Sendable` ([Track]) async -> LyricsResult + private let impl = LyricsUseCaseImpl() func fetchLyrics(track: Track) async -> LyricsResult { await handler([track]) } func fetchLyrics(candidates: [Track]) async -> LyricsResult { await handler(candidates) } func parseLyricsContent(from result: LyricsResult?) -> LyricsContent? { - LyricsUseCaseImpl().parseLyricsContent(from: result) + impl.parseLyricsContent(from: result) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/TrackHandlerTests/TrackHandlerImplTests.swift` around lines 176 - 183, The test stub StubLyricsUseCase currently calls LyricsUseCaseImpl().parseLyricsContent(from:) each time; keep this delegation but avoid repeated allocations by creating and reusing a single LyricsUseCaseImpl instance inside the stub (e.g., store a private let impl = LyricsUseCaseImpl() and call impl.parseLyricsContent(from:) from parseLyricsContent(from:)), ensuring StubLyricsUseCase still conforms to LyricsUseCase and preserves existing fetchLyrics behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Tests/TrackHandlerTests/TrackHandlerImplTests.swift`:
- Around line 176-183: The test stub StubLyricsUseCase currently calls
LyricsUseCaseImpl().parseLyricsContent(from:) each time; keep this delegation
but avoid repeated allocations by creating and reusing a single
LyricsUseCaseImpl instance inside the stub (e.g., store a private let impl =
LyricsUseCaseImpl() and call impl.parseLyricsContent(from:) from
parseLyricsContent(from:)), ensuring StubLyricsUseCase still conforms to
LyricsUseCase and preserves existing fetchLyrics behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cae049d-69e8-46bb-94e0-8e5e17da6f5f
📒 Files selected for processing (14)
Sources/LyricsUseCase/LyricsUseCaseImpl.swiftTests/AsyncRunnableCommandTests/AsyncRunnableCommandTests.swiftTests/ConfigDataSourceTests/SanitizeTomlFloatsTests.swiftTests/ConfigDataSourceTests/TryDecodeTests.swiftTests/ConfigHandlerTests/ConfigHandlerImplTests.swiftTests/HealthHandlerTests/HealthHandlerImplTests.swiftTests/LyricsUseCaseTests/LyricsContentParsingTests.swiftTests/ProcessHandlerTests/ProcessHandlerImplTests.swiftTests/ProcessHandlerTests/ProcessLockTests.swiftTests/SQLiteDataStoreTests/DatabaseManagerMigrationTests.swiftTests/ServiceHandlerTests/ServiceHandlerImplTests.swiftTests/TrackHandlerTests/TrackHandlerImplTests.swiftTests/VersionHandlerTests/VersionHandlerImplTests.swiftTests/WallpaperInteractorTests/WallpaperInteractorImplTests.swift
✅ Files skipped from review due to trivial changes (10)
- Tests/VersionHandlerTests/VersionHandlerImplTests.swift
- Tests/ServiceHandlerTests/ServiceHandlerImplTests.swift
- Tests/ProcessHandlerTests/ProcessHandlerImplTests.swift
- Tests/ProcessHandlerTests/ProcessLockTests.swift
- Tests/ConfigHandlerTests/ConfigHandlerImplTests.swift
- Tests/HealthHandlerTests/HealthHandlerImplTests.swift
- Tests/AsyncRunnableCommandTests/AsyncRunnableCommandTests.swift
- Tests/WallpaperInteractorTests/WallpaperInteractorImplTests.swift
- Tests/ConfigDataSourceTests/SanitizeTomlFloatsTests.swift
- Tests/LyricsUseCaseTests/LyricsContentParsingTests.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Sources/LyricsUseCase/LyricsUseCaseImpl.swift
- Tests/SQLiteDataStoreTests/DatabaseManagerMigrationTests.swift
- Tests/ConfigDataSourceTests/TryDecodeTests.swift
概要
Entity からロジックを Impl に移植し、カバレッジの低いモジュールにテストを追加。テスト数 426 → 452（+26）。
リファクタ
LyricsContent.init(from:)+parseSyncedLyrics→LyricsUseCaseImpl.parseLyricsContent(from:)FetchStatecomputed properties →Presenters/FetchState+Convenience.swiftテスト追加
Part of #172
テスト計画
swift test全452テスト合格（既存 flaky 1件除く）Summary by CodeRabbit
New Features
Tests
Refactor