Run all tests #2979
Merged
Run all tests #2979
Conversation
This file contains 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
Previously CI was *running* tests but ignoring failures. This change makes the build fail on failures.
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.
Looks like this has been commented out since b484032
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.
For some reason CI doesn't seem to like using `TextEdit.app`.
This reverts commit 154ca03. Apparently having the application available was not the issue.
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.
not a target dependency.
Add dependencies to Quicksilver Tests as needed
Still build a `Testing` copy, which allows one to start monitoring this target in Instruments.app / profiling / debuggers *before any tests have run*.
Little reason not to test signing the artifacts in CI which should help alert us to issues in between releases.
The comment reports it will fall back to bezel but then tries to use Primer -- this fixes that.
Unclear why this fixes the test on my machine. But it does.
`QS_BUILD_ONLY`: Just build, don't test or sign `QS_DONT_SIGN`: Build and test but don't sign
|
@pjrobertson @skurfer I'd like to merge this soon, if you get a chance to take a look, so I can move on to several of the Sonoma bugs, while hopefully ensuring that the fixes aren't unintentionally resulting in backwards incompatible changes. |
Scanning HOME triggers blocking permissions popups that can block further execution until manual intervention.
|
Merging this in to avoid rebase hell. |
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.
Fixes #2971
This PR includes a number of changes mostly to the tests, CI, and build toolchain.
It does add a few
ifdef TESTINGto avoid things like the accessibility checker on CI, which otherwise blocks forever and then fails.It add a new plist file
QSCorePlugIn-Info-Testing.plistto try to avoid indexing "sensitive" directories like~/Desktopand~/Documentson startup, which also result in popups that can block further execution.In several places the default build interface was supposed(?) to be Bezel but was set to Primer; this changes it to Bezel.
It changes several build steps to allow automatic dependency resolution to work, specifically by removing dependencies like the Bezel interface from the
Quicksilvertarget (that I think I put there in the first place) and putting them underQuicksilver Tests. (They are still underQuicksilver Distribution, so I don't think this should change peoples dev workflow.) It also enables parallel testing, which in addition to parallel builds really speeds up the build process on my machine, and configures the testing order to be randomized, to hopefully avoid issues with race conditions between tests that just happen to work in our favor when build alphabetically.There are a lot of commits, several of which could be dropped (later reverted), but as I went through rebasing and squashing I decided it would be best to leave them separate to enable bisecting in the future if any of them end up being problematic (as opposed to squashing them together and trying to tease the changes apart later).
Finally, this adds a nice little build status badge to the readme and removes a few whitespace errors.