test: wait for ShipIt to exit before asserting on install result#319
Merged
Conversation
installWithRequest:remote:YES previously returned immediately after SMJobSubmit, leaving callers to poll testApplicationBundleVersion with Nimble's default 1s toEventually timeout. On slow CI hosts that races launchd spawn + codesign verify + bundle rename and intermittently loses (observed on the macos-15-intel runner). Split the helper into submitShipItRequest: (fire-and-forget, used by the signal-handling tests that need to interrupt ShipIt mid-install) and waitForShipItJobToExitWithLabel:, which polls SMJobCopyDictionary for LastExitStatus. installWithRequest:remote:YES now calls both, so its callers can assert synchronously with .to(equal:). The SQRLUpdaterSpec end-to-end tests that launch TestApplication and let it submit its own ShipIt job get the same wait with the TestApplication-derived job label. The remaining toEventually calls (relaunch detection via runningApplicationsWithBundleIdentifier:) are genuinely async and now use SQRLLongTimeout explicitly.
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
1 participant
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.
-installWithRequest:remote:YESpreviously returned immediately afterSMJobSubmit, leaving callers to polltestApplicationBundleVersionwith Nimble's default 1-secondtoEventuallytimeout. On slow CI hosts (themacos-15-intelrunner in particular) that races launchd spawn → codesign verify → bundle rename and intermittently loses, surfacing asexpected to eventually equal <2.1>, got <1.0>.This isn't a retry — the timeout was objectively wrong for an out-of-process install. Two of the eleven call sites had already worked around it locally with
withTimeout(SQRLLongTimeout); this makes the wait deterministic at the source instead.Changes:
-submitShipItRequest:(fire-and-forget — the signal-handling tests need to interrupt ShipIt mid-install) and-waitForShipItJobToExitWithLabel:(pollsSMJobCopyDictionaryforLastExitStatus, which only appears after the process has actually run and exited — avoids the brief no-PID window before launchd spawns it).-installWithRequest:remote:YESnow calls both, so its callers assert synchronously with.to(equal:).SQRLUpdaterSpecend-to-end tests that launchTestApplicationand let it submit its own ShipIt job get the same wait with the TestApplication-derived label.toEventuallycalls (relaunch detection viarunningApplicationsWithBundleIdentifier:) are genuinely async and now useSQRLLongTimeoutexplicitly.44/44 across
SQRLInstallerSpec+SQRLUpdaterSpeclocally.