Add unit tests for config models, defaults/validation, camera identity mapping#93
Open
kmatzen wants to merge 4 commits into
Open
Add unit tests for config models, defaults/validation, camera identity mapping#93kmatzen wants to merge 4 commits into
kmatzen wants to merge 4 commits into
Conversation
…ntity mapping Covers previously-untested pure-logic modules: CameraConfig/GoProSettingsData Codable round-trips, DefaultConfigurations preset values, ConfigValidation (including hasSettingsMismatch), and CameraIdentityManager/CameraSerialResolver name and UUID persistence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LgiY7AhBXZjExzafhAp5
The explicit -only-testing list silently skipped any newly added test class (including the ones just added), so new tests weren't actually exercised by CI until someone remembered to list them by name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LgiY7AhBXZjExzafhAp5
Both ranges were stale relative to the actual value domains used elsewhere in the app: mode holds raw GoPro camera mode IDs (up to 27, see CameraSettingDescriptions.modeDescription; FacettApp.swift sets mode = 12 for video), and lcdBrightness is a 0-100 percentage (see lcdBrightnessProfileDescription). The stale 0...1 / 0...2 ranges meant ConfigValidation.validateConfig flagged the app's own Default config (mode: 12, lcdBrightness: 100) as invalid — caught by the new ConfigValidationTests added in this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LgiY7AhBXZjExzafhAp5
SettingsValidator.ValidRanges is stale for several more fields beyond mode/lcdBrightness (antiFlicker, rawAudio, hindsight, whiteBalance, ...), several of which are sparse non-contiguous ID sets that a ClosedRange can't represent correctly regardless of bounds. That's a larger, separate fix - tracked in #94. Narrow this test to check validateConfig's plumbing (isValid matches errors.isEmpty) without asserting the current, false invariant that defaults pass validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LgiY7AhBXZjExzafhAp5
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
2 participants
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.
Summary
ConfigurationUtilsTests.swiftcoveringDefaultConfigurationspreset values,fpsDescription, andConfigValidation(includinghasSettingsMismatchfield-diff, shutter-exclusion, and encoding-skip behavior)CameraConfigModelsTests.swiftcoveringGoProSettingsDataJSON round-trips /GoProSettingsconversion andCameraConfiginit variants + Codable round-tripCameraIdentityManagerTests.swiftcoveringCameraIdentityManagername storage/fallback andCameraSerialResolverUUID mappingbuild.yml) to run the fullFacettTestsbundle instead of an explicit per-class-only-testingallowlist, so newly added test classes (including these) actually execute instead of being silently skippedTest plan
project.pbxprojchanges needed)macos-15runner) builds and runsFacettTestson this PR — verify it's greenGenerated by Claude Code