tests: prevent UserConfigTests from touching real user config directory#313
Open
rahulxsomething wants to merge 1 commit intomikker:mainfrom
Open
tests: prevent UserConfigTests from touching real user config directory#313rahulxsomething wants to merge 1 commit intomikker:mainfrom
rahulxsomething wants to merge 1 commit intomikker:mainfrom
Conversation
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
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
This PR fixes a high-severity test isolation bug where
UserConfigTestscould read/write/delete files in the real user config location (~/Library/Application Support/Leader Key).Some tests exercised the “default config directory” path and could remove that directory or write invalid JSON into the live
config.jsonduring normalxcodebuild testruns.Root Cause
UserConfigTestsused real default-directory resolution when validating fallback behavior. That allowed test file operations to target real user data.Changes
1) Add a narrow injection seam for default directory resolution
Leader Key/UserConfig.swiftUserConfignow accepts an init parameter:defaultDirectoryResolver: @escaping () -> String = UserConfig.defaultDirectoryensureValidConfigDirectory()now uses this resolver.2) Fully isolate
UserConfigTestsLeader KeyTests/UserConfigTests.swiftsetUp, tests now provide an isolated temp default directory via the new init resolver.Safety / Compatibility
Validation
xcodebuild -scheme 'Leader Key' -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO testxcodebuild -scheme 'Leader Key' -configuration Release -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO buildBoth succeeded.
Additionally verified manually that the real config file hash at
~/Library/Application Support/Leader Key/config.jsonstayed unchanged before/after test and build runs.Why this should merge quickly