Skip to content

tests: prevent UserConfigTests from touching real user config directory#313

Open
rahulxsomething wants to merge 1 commit intomikker:mainfrom
rahulxsomething:fix/test-isolate-config-tests-squash
Open

tests: prevent UserConfigTests from touching real user config directory#313
rahulxsomething wants to merge 1 commit intomikker:mainfrom
rahulxsomething:fix/test-isolate-config-tests-squash

Conversation

@rahulxsomething
Copy link

Summary

This PR fixes a high-severity test isolation bug where UserConfigTests could 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.json during normal xcodebuild test runs.

Root Cause

UserConfigTests used 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.swift
  • UserConfig now accepts an init parameter:
    • defaultDirectoryResolver: @escaping () -> String = UserConfig.defaultDirectory
  • ensureValidConfigDirectory() now uses this resolver.
  • Runtime behavior is unchanged because production code uses the default resolver.

2) Fully isolate UserConfigTests

  • Leader KeyTests/UserConfigTests.swift
  • In setUp, tests now provide an isolated temp default directory via the new init resolver.
  • Tests no longer call real-path default resolution when validating fallback/parse-error behavior.
  • Assertions were updated accordingly.

Safety / Compatibility

  • No config format changes.
  • No migration needed.
  • No user-facing behavior changes expected.

Validation

  • xcodebuild -scheme 'Leader Key' -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO test
  • xcodebuild -scheme 'Leader Key' -configuration Release -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO build

Both succeeded.

Additionally verified manually that the real config file hash at ~/Library/Application Support/Leader Key/config.json stayed unchanged before/after test and build runs.

Why this should merge quickly

  • Small, focused diff (2 files).
  • Addresses user-data safety risk in test runs.
  • Keeps production behavior unchanged.

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

Comments