Reliable dealloc checks, real tests and CI (3.1) - #18
Open
DanielCech wants to merge 1 commit into
Open
DanielCech wants to merge 1 commit into
DanielCech wants to merge 1 commit into
Conversation
- Fix hang on macOS, crash on nil objectCreation, hang on non-testable objects - Detect leaks per instance instead of per class - Replace fixed sleeps with polling up to deallocationTimeout - Create presenting controller automatically, clean up window in tearDown - Thread-safe dealloc registry; fix associated object key warnings - Use explicit DEALLOC_TESTS_DI flag instead of canImport - Use https URL for ios-dependency-injection - Drop Sendable requirement from DeallocTestable - Logging opt-in via DeallocTester.isLoggingEnabled - Deprecate DefaultInitializable - Add XCTest suites for both products and GitHub Actions CI - Remove Travis, Danger, Carthage, jazzy and unused headers - Fix DIFree sample for Swift 6, rewrite README Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Why
DeallocTests has been in use for a long time, but a review showed that some of its checks could silently give the wrong answer, a few situations made tests hang or crash, and the repository still carried tooling from the Carthage/Travis era. This PR fixes those problems without changing how you write dealloc tests. Existing test code keeps compiling.
What was wrong, and what changed
Tests could hang or crash
objectCreationreturnednil(easy to hit with[weak self]), the test crashed. If the object wasn'tDeallocTestable, it hung. Both now fail with a clear message, and the scenario continues with the next step.showPresentingController()crashed when testing a view controller. The presenting controller is now created automatically, and the test window is cleaned up afterwards.Some leaks went unnoticed
FooViewControllerwas freed and another leaked, the test passed. Leaks are now tracked per instance.Tests were slower than needed
deallocationTimeout(2 s by default). In the sample app, the full coordinator scenario now takes about 2 s, and nearly all of that is waiting on the intentional leak.Installing the package could fail
ios-dependency-injectionused an SSH URL, so anyone without SSH access to GitHub (including many CI machines) couldn't resolve the package. EvenDeallocTestsDIFreeusers were affected. It now uses https.DeallocTestsDIFreerelied oncanImport(DependencyInjection), which depends on what else happens to be in the build. The DI variant now uses an explicit compile flag, and tests confirm that the DIFree variant builds without DI.Smaller improvements
DeallocTestableno longer requiresSendable. Retroactive conformances likeextension MyViewController: @retroactive DeallocTestable {}no longer produce warnings.Alloc/Deallocconsole output is now off by default. Turn it on withDeallocTester.isLoggingEnabled = true.setUp()can now be overridden in your subclasses (it waspublic, nowopen).DefaultInitializableis marked deprecated. It isn't related to dealloc testing and will be removed in 4.0.Tests and CI
nilfactory, a non-testable object,checkClasses,actionBeforeCheck, and shared instances from the DI container. The old empty Quick/Nimble spec didn't build and has been removed.swift teston macOS and builds both sample apps. The samples contain an intentional leak, so CI only compiles them and doesn't run their tests.Cleanup
.swift-version,.ruby-version, unused Objective-C headers and an empty source file.Behaviour changes worth noting in release notes
deallocationTimeout(2 s) instead of the old fixed delays.How it was verified
swift teston macOS: 11 tests pass. Expected failures are asserted withXCTExpectFailure.SecondViewController, and all other screens and the coordinator pass.macos-15(Xcode 16) will be the first check on older toolchains.Next steps (not in this PR)
A follow-up 4.0 could add a Swift Testing API, drop the need for
DeallocTestableconformances by tracking objects with plain weak references, report which property still holds a leaked object, and replace the two products with a package trait for dependency injection.🤖 Generated with Claude Code