Skip to content

Commit

Permalink
Run all tests (#2979)
Browse files Browse the repository at this point in the history
* Really run tests

Previously CI was *running* tests but ignoring failures. This change
makes the build fail on failures.

* Objects can't be equal if they have differing names, now can they?

* Compare objects with `is[Not]EqualTo:`, not `[=!]=`

* Don't automatically index private directories when testing

By default, `QSCorePlugIn-Info.plist` is configured to index directories
that newer editions of MacOS consider "sensitive," such as:
- `~/Desktop`
- `~/Downloads`
- `~/Documents`

Accessing these directories *immediately* provokes a permissions popup
that **blocks** until dismissed or accepted.

Unfortunately, this is problematic for testing (and especially headless
testing / CI) in that the tests block -- indefinitely -- until this
popup is acted on, whether or not these directories are actually
required for the test.

Thankfully none of our existing tests necessarily require access to any
of these directories. This commit provides an alternative file
(`QSCorePlugIn-Info-Testing.plist`) to be used only during the Testing
scheme that does *not* index these directories and thereby avoids this
problematic default. One test was changed to as to not access
`~/Downloads`, which seemed unnecessary for the function being tested.

Additionally, this commit sets the `QSDisableExternalPlugIns`
environment variable -- only for the Testing scheme -- which prevents
automatic loading of plugins in `~/Library/Application
Support/Quicksilver/PlugIns` (among other locations). For
reproducibility, these user-installed plugins should probably *not* be
automatically included during testing. For example, having the
`AddressBook` plugin enabled will trigger a permissions popup (similar
to those described above) at launch time, blocking the test
indefinitely, even though it is not required for the Quicksilver test
suite.

* Remove unused build phase

Looks like this has been commented out since b484032

* Resolve build / test time race condition

This seems to resolve a long-time thorn-in-the-side, where XCode fails
to build Quicksilver from scratch the first time, then succeeds upon a
second (or maybe third) try. See also:

- #332
- #2583 (comment)

The related failures can usually be traced back to a missing
`/tmp/QS/Configuration/Quicksilver.pch`.

It seems that the existing `Copy Files` build step in `Preflight`, which
copies the `Configuration` directory, doesn't specify `Quicksilver.pch`
explicitly (instead copying the entire directory). Unfortunately XCode
doesn't seem smart enough to realize that the directory contents may be
required for downstream dependencies and therefore doesn't take it into
consideration for automatic dependency ordering.

Adding a second noop build step that *specifies this output file*
(`Quicksilver.pch`) clues XCode in and seems to resolve the issues with
the automatic dependency detection.

Several ways to skin this cat, I chose this route because shell scripts
can contain comments, so I can warn future devs not to remove the
"empty" step.

* What applications are available?

* Remove debug print

* Test on QS itself

For some reason CI doesn't seem to like using `TextEdit.app`.

* Revert "Test on QS itself"

This reverts commit 154ca03.

Apparently having the application available was not the issue.

* Cleanup

* Refactor, test configuration Testing

* Remove extra `\n`

* Use implicit dependency order everywhere

* Use the `Testing` configuration for all tests

* Debug test path

* Force path

* Update GA runner

* keyEventWithType:10 -> keyEventWithType:NSEventTypeKeyDown

* Give better name

* Rename test plan

* Disable loading external plugins when testing

* Remove `Quicksilver` dependendy from `Core Support`

This leads to a "cycle in dependencies" issue due to the `BUNDLE_LOADER`
setting. `Core Support` doesn't depend on QS.

This also adds Core Support as a dependency of `Quicksilver Tests`,
which *is* needed.

* Unset `BUNDLE_LOADER`, which otherwise gives a linker error if `QS` is
not a target dependency.

* Test for nil interface controller

* Avoid cyclic dependencies between QS and plugins

Add dependencies to Quicksilver Tests as needed

* Don't build external plugins during testing

* Randomize execution order

* Don't show accessibility prompt during testing

* Skip tests if running with `QS_BUILD_ONLY`

Still build a `Testing` copy, which allows one to start monitoring this
target in Instruments.app / profiling / debuggers *before any tests have
run*.

* Run signing steps in CI

Little reason not to test signing the artifacts in CI which should help
alert us to issues in between releases.

* Try MacOS 13

* Specify and show xcode version

* Select command line tools

* Switch command line tools

* Revert back to MacOS 12?

* Revert setting xcode version

* Default interface is primer, not bezel

* Don't show setup assistant if testing

* Fallback / default is bezel

The comment reports it will fall back to bezel but then tries to use
Primer -- this fixes that.

* Remove activate:nil to fix test

Unclear why this fixes the test on my machine. But it does.

* Test on bezel, not primer

* Add a separate envvar for signing vs build_only

`QS_BUILD_ONLY`: Just build, don't test or sign
`QS_DONT_SIGN`: Build and test but don't sign

* Add build status badge

* Don't scan HOME when `Testing`

Scanning HOME triggers blocking permissions popups that can block
further execution until manual intervention.
  • Loading branch information
n8henrie authored Nov 24, 2023
1 parent 2ea128e commit 5a5bce4
Show file tree
Hide file tree
Showing 28 changed files with 2,508 additions and 144 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ on:

jobs:
build:
runs-on: macos-11
runs-on: macos-12
env:
QS_BUILD_ONLY: 1
QS_DONT_SIGN: 1
steps:
- uses: actions/checkout@v3
with:
Expand Down
9 changes: 9 additions & 0 deletions Quicksilver/Code-App/QSController.m
Original file line number Diff line number Diff line change
Expand Up @@ -708,8 +708,13 @@ - (void)checkForFirstRun {
default: // QSApplicationNormalLaunch:
break;
}

// Don't block the interface with the setup assistant if running a test build
#ifndef TESTING
if (![defaults boolForKey:kSetupAssistantCompleted] || lastVersion <= [@"3694" hexIntValue] || ![defaults boolForKey:@"QSAgreementAccepted"])
runningSetupAssistant = YES;
#endif

#ifndef DEBUG
[NSApp updateLaunchStatusInfo];
#endif
Expand Down Expand Up @@ -804,6 +809,10 @@ - (void)relaunchQuicksilver
# pragma mark - Accessibility Permissions

-(BOOL)checkForAccessibilityPermission {
#ifdef TESTING
return YES;
#endif

// Prompt for accessibility permissions on macOS Mojave and later.
if (!accessibilityChecker) {
accessibilityChecker = [NSTimer scheduledTimerWithTimeInterval:1 repeats:YES block:^(NSTimer * _Nonnull timer) {
Expand Down
4 changes: 2 additions & 2 deletions Quicksilver/Code-QuickStepCore/QSInterfaceMediator.m
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
@implementation QSRegistry (QSCommandInterface)
- (NSString *)preferredCommandInterfaceID {
NSString *key = [[NSUserDefaults standardUserDefaults] stringForKey:kQSCommandInterfaceControllers];
if (![[self tableNamed:kQSCommandInterfaceControllers] objectForKey:key]) key = @"QSPrimerInterfaceController";
if (![[self tableNamed:kQSCommandInterfaceControllers] objectForKey:key]) key = @"QSBezelInterfaceController";
return key;
}

Expand All @@ -23,7 +23,7 @@ - (QSInterfaceController*)preferredCommandInterface {
[prefInstances setObject:mediator forKey:kQSCommandInterfaceControllers];
} else {
QSShowNotifierWithAttributes([NSDictionary dictionaryWithObjectsAndKeys:@"QSNotification", QSNotifierType, [QSResourceManager imageNamed:kQSBundleID], QSNotifierIcon, NSLocalizedString(@"Interface Changed", nil), QSNotifierTitle, NSLocalizedString(@"Interface could not be loaded. Switching to Bezel",nil), QSNotifierText, nil]);
mediator = [self instanceForKey:@"QSPrimerInterfaceController" inTable:kQSCommandInterfaceControllers];
mediator = [self instanceForKey:@"QSBezelnterfaceController" inTable:kQSCommandInterfaceControllers];
[prefInstances setObject:mediator forKey:kQSCommandInterfaceControllers];
}
}
Expand Down
4 changes: 2 additions & 2 deletions Quicksilver/Code-QuickStepCore/QSObject.m
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ - (BOOL)isEqual:(QSObject *)anObject {
}
}
// test the meta dictionary for QSObjectName, QSObjectLabel - if they're not equal then return no
if ([meta objectForKey:kQSObjectAlternateName] != [anObject->meta objectForKey:kQSObjectAlternateName] ||
[meta objectForKey:kQSObjectPrimaryName] != [anObject->meta objectForKey:kQSObjectPrimaryName] ) {
if ([[meta objectForKey:kQSObjectAlternateName] isNotEqualTo:[anObject->meta objectForKey:kQSObjectAlternateName]] ||
[[meta objectForKey:kQSObjectPrimaryName] isNotEqualTo:[anObject->meta objectForKey:kQSObjectPrimaryName]] ) {
return NO;
}
return YES;
Expand Down
Loading

0 comments on commit 5a5bce4

Please sign in to comment.