|
| 1 | +# Testing swift-corelibs-foundation |
| 2 | + |
| 3 | +swift-corelibs-foundation uses XCTest for its own test suite. This document explains how we use it and how we organize certain kinds of specialized testing. This is both different from the Swift compiler and standard library, which use `lit.py`, and from destkop testing, since the version of XCTest we use is not the Darwin one, but the Swift core library implementation in `swift-corelibs-xctest`, which is pretty close to the original with some significant differences. |
| 4 | + |
| 5 | +## Tests Should Fail, Not Crash |
| 6 | + |
| 7 | +### In brief |
| 8 | + |
| 9 | +* Tests should fail rather than crashing; swift-corelibs-xctest does not implement any crash recovery |
| 10 | +* You should avoid forced optional unwrapping (e.g.: `aValue!`). Use `try aValue.unwrapped()` instead |
| 11 | +* You can test code that is expected to crash; you must mark the whole body of the test method with `assertCrashes(within:)` |
| 12 | +* If a test or a portion of a test is giving the build trouble, use `testExpectedToFail` and write a bug |
| 13 | + |
| 14 | +### Why and How |
| 15 | + |
| 16 | +XCTest on Darwin can implement a multiprocess setup that allows a test run to continue if the test process crashes. On Darwin, code is built into a bundle, and a specialized tool called `xctest` runs the test by loading the bundle; the Xcode infrastructure can detect the crash and restart the tool from where it left off. For swift-corelibs-xctest, instead, the Foundation test code is compiled into a single executable and that executable is run by the Swift build process; if it crashes, subsequent tests aren't run, which can mask regressions that are merged while the crash is unaddressed. |
| 17 | + |
| 18 | +Due to this, it is important to avoid crashing in test code, and to properly handle tests that do. Every API is unique in this regard, but some situations are common across tests. |
| 19 | + |
| 20 | +#### Avoiding Forced Unwrapping |
| 21 | + |
| 22 | +Forced unwrapping is easily the easiest way to crash the test process, and should be avoided. We have an ergonomic replacement in the form of the `.unwrapped()` extension method on the `Optional` type. |
| 23 | + |
| 24 | +The following code is a liability and code review should flag it: |
| 25 | + |
| 26 | +```swift |
| 27 | +func testSomeInterestingAPI() { |
| 28 | + let x = interestingAPI.someOptionalProperty! // <<< Incorrect! |
| 29 | + |
| 30 | + XCTAssertEqual(x, 42, "The correct answer is present") |
| 31 | +} |
| 32 | +``` |
| 33 | + |
| 34 | +Instead: |
| 35 | + |
| 36 | +1. Change the test method to throw errors by adding the `throws` clause. Tests that throw errors will fail and stop the first time an error is thrown, so plan accordingly, but a thrown error will not stop the test run, merely fail this test. |
| 37 | +2. Change the forced unwrapping to `try ….unwrapped()`. |
| 38 | + |
| 39 | +For example, the code above can be fixed as follows: |
| 40 | + |
| 41 | +```swift |
| 42 | +func testSomeInterestingAPI() throws { // Step 1: Add 'throws' |
| 43 | + // Step 2: Replace the unwrap. |
| 44 | + let x = try interestingAPI.someOptionalProperty.unwrapped() |
| 45 | + |
| 46 | + XCTAssertEqual(x, 42, "The correct answer is present") |
| 47 | +} |
| 48 | +``` |
| 49 | + |
| 50 | +#### Asserting That Code Crashes |
| 51 | + |
| 52 | +Some API, like `NSCoder`'s `raiseException` failure policy, are _supposed_ to crash the process when faced with edge conditions. Since tests should fail and not crash, we have been unable to test this behavior for the longest time. |
| 53 | + |
| 54 | +Starting in swift-corelibs-foundation in Swift 5.1, we have a new utility function called `assertCrashes(within:)` that can be used to indicate that a test crashes. It will respawn a process behind the scene, and fail the test if the second process doesn't crash. That process will re-execute the current test, _including_ the contents of the closure, up to the point where the first crash occurs. |
| 55 | + |
| 56 | +To write a test function that asserts some code crashes, wrap its **entire body** as in this example: |
| 57 | + |
| 58 | +```swift |
| 59 | +func testRandomClassDoesNotDeserialize() { |
| 60 | + assertCrashes { |
| 61 | + let coder = NSKeyedUnarchiver(requiresSecureCoding: false) |
| 62 | + coder.requiresSecureCoding = true |
| 63 | + coder.decodeObject(of: [AClassThatIsntSecureEncodable.self], forKey: …) |
| 64 | + … |
| 65 | + } |
| 66 | +} |
| 67 | +``` |
| 68 | + |
| 69 | +Since the closure will only execute to the first crash, ensure you do not use multiple `assertCrashes…` markers in the same test method, that you do _not_ mix crash tests with regular test code, and that if you want to test multiple crashes you do so with separate test methods. Wrapping the entire method body is an easy way to ensure that at least some of these objectives are met. |
| 70 | + |
| 71 | +#### Stopping Flaky or Crashing Tests |
| 72 | + |
| 73 | +A test that crashes or fails multiple times can jeopardize patch testing and regression reporting. If a test is flaky or outright failing or crashing, it should be marked as expected to fail ASAP using the appropriate Foundation test utilities. |
| 74 | + |
| 75 | +Let's say a test of this form is committed: |
| 76 | + |
| 77 | +```swift |
| 78 | +func testNothingUseful() { |
| 79 | + fatalError() // Smash the machine! |
| 80 | +} |
| 81 | +``` |
| 82 | + |
| 83 | +A test fix commit should be introduced that does the following: |
| 84 | + |
| 85 | +* Write a bug to investigate and re-enable the test on [the Swift Jira instance](https://bugs.swift.org/). Have the link to the bug handy (e.g.: `http://bugs.swift.org/browse/SR-999999`). |
| 86 | + |
| 87 | +* Find the `allTests` entry for the offending test. For example: |
| 88 | + |
| 89 | +```swift |
| 90 | +var allTests: […] { |
| 91 | + return [ |
| 92 | + // … |
| 93 | + ("testNothingUseful", testNothingUseful), |
| 94 | + // … |
| 95 | + ] |
| 96 | +``` |
| 97 | + |
| 98 | +* Replace the method reference with a call to `testExpectedToFail` that includes the reason and the bug link. Mark that location with a `/* ⚠️ */` comment. For example: |
| 99 | + |
| 100 | +```swift |
| 101 | +var allTests: […] { |
| 102 | + return [ |
| 103 | + // … |
| 104 | + // Add the prefix warning sign and the call: |
| 105 | + /* ⚠️ */ ("testNothingUseful", testExpectedToFail(testNothingUseful, |
| 106 | + "This test crashes for no clear reason. http://bugs.swift.org/browse/SR-999999")), |
| 107 | + // … |
| 108 | + ] |
| 109 | +``` |
| 110 | + |
| 111 | +Alternately, let's say only a portion of a test is problematic. For example: |
| 112 | + |
| 113 | +```swift |
| 114 | +func testAllMannersOfIO() throws { |
| 115 | + try runSomeRockSolidIOTests() |
| 116 | + try runSomeFlakyIOTests() // These fail pretty often. |
| 117 | + try runSomeMoreRockSOlidIOTests() |
| 118 | +} |
| 119 | +``` |
| 120 | + |
| 121 | +In this case, file a bug as per above, then wrap the offending portion as follows: |
| 122 | + |
| 123 | +```swift |
| 124 | +func testAllMannersOfIO() throws { |
| 125 | + try runSomeRockSolidIOTests() |
| 126 | + |
| 127 | + /* ⚠️ */ |
| 128 | + if shouldAttemptXFailTests("These fail pretty often. http://bugs.swift.org/browse/SR-999999") { |
| 129 | + try runSomeFlakyIOTests() |
| 130 | + } |
| 131 | + /* ⚠️ */ |
| 132 | + |
| 133 | + try runSomeMoreRockSOlidIOTests() |
| 134 | +} |
| 135 | +``` |
| 136 | + |
| 137 | +Unlike XFAIL tests in `lit.py`, tests that are expected to fail will _not_ execute during the build. If your test is disabled in this manner, you should investigate the bug by running the test suite locally; you can do so without changing the source code by setting the `NS_FOUNDATION_ATTEMPT_XFAIL_TESTS` environment variable set to the string `YES`, which will cause tests that were disabled this way to attempt to execute anyway. |
| 138 | + |
| 139 | +## Test Internal Behavior Carefully: `@testable import` |
| 140 | + |
| 141 | +### In brief |
| 142 | + |
| 143 | +* Prefer black box (contract-based) testing to white box testing wherever possible |
| 144 | +* Some contracts cannot be tested or cannot be tested reliably (for instance, per-platform fallback paths); it is appropriate to use `@testable import` to test their component parts instead |
| 145 | +* Ensure your test wraps any reference to `internal` functionality with `#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT` |
| 146 | +* Ensure the file you are using this in adds a `@testable import` prologue |
| 147 | +* Run with this enabled _and_ disabled prior to PR |
| 148 | + |
| 149 | +### Why and How |
| 150 | + |
| 151 | +In general, we want to ensure that tests are written to check the _contract_ of the API, [as documented for each class](https://developer.apple.com/). It is of course acceptable to have the test implementation be informed by the implementation, but we want to make sure that tests still make sense if we replace an implementation entirely, [as we sometimes do](https://github.com/apple/swift-corelibs-foundation/pull/2331). |
| 152 | + |
| 153 | +This doesn't always work. Sometimes the contract specifies that a certain _result_ will occur, and that result may be platform-specific or trigger in multiple ways, all of which we'd like to test (for example, different file operation paths for volumes with different capabilities). In this case, we can reach into Foundation's `internal` methods by using `@testable import` and test the component parts or invoke private API ("SPI") to alter the behavior so that all paths are taken. |
| 154 | + |
| 155 | +If you think this is the case, you must be careful. We run tests both against a debug version of Foundation, which supports `@testable import`, and the release library, which does not. Your tests using `internal` code must be correctly marked so that tests don't succeed in one configuration but fail in the other. |
| 156 | + |
| 157 | +To mark those tests: |
| 158 | + |
| 159 | +* Wrap code using internal features with `#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT`: |
| 160 | + |
| 161 | +```swift |
| 162 | +func testSomeFeature() { |
| 163 | +#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT |
| 164 | + try Date._someInternalTestModifierMethod { |
| 165 | + // … |
| 166 | + } |
| 167 | +#endif |
| 168 | +} |
| 169 | +``` |
| 170 | + |
| 171 | +* In the file you're adding the test, if not present, add the appropriate `@testable import` import prologue at the top. It will look like the one below, but **do not** copy and paste this — instead, search the codebase for `@testable import` for the latest version: |
| 172 | + |
| 173 | +```swift |
| 174 | +// It will look something like this: |
| 175 | + |
| 176 | +#if NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT |
| 177 | + #if canImport(SwiftFoundation) && !DEPLOYMENT_RUNTIME_OBJC |
| 178 | + @testable import SwiftFoundation |
| 179 | + … |
| 180 | +``` |
| 181 | + |
| 182 | +* Run your tests both against a debug Foundation (which has testing enabled) and a release Foundation (which does not). If you're using `build-script` to build, you can produce the first by using the `--debug-foundation` flag and the latter with the regular `--foundation` flag. **Do this before creating a PR.** Currently the pipeline checks both versions only outside PR testing, and we want to be sure that the code compiles in both modes before accepting a patch like this. (Automatic testing and dual-mode PR testing are forthcoming.) |
| 183 | + |
| 184 | +## Testing NSCoding: Don't Write This From Scratch; Use Fixtures |
| 185 | + |
| 186 | +### In brief |
| 187 | + |
| 188 | +* We want `NSCoding` to work with archives produced by _both_ Darwin Foundation and swift-corelibs-foundation |
| 189 | +* Where possible, _do not_ write your own coding test code — use the fixture infrastructure instead |
| 190 | +* Fixture tests will both test roundtrip coding (reading back what swift-corelibs-foundation produces), and archive coding (with archives produced by Darwin Foundation) |
| 191 | +* Add a fixture test by adding fixtures to `FixtureValues.swift` |
| 192 | +* Use `assertValueRoundtripsInCoder(…)` and `assertLoadedValuesMatch(…)` in your test methods |
| 193 | +* Use the `GenerateTestFixtures` project in the `Tools` directory to generate archives for `assertLoadedValuesMatch(…)`, and commit them in `TestFoundation/Fixtures`. |
| 194 | +* Please generate your fixtures on the latest released (non-beta) macOS. |
| 195 | + |
| 196 | +### Why and How |
| 197 | + |
| 198 | +`NSCoding` in swift-corelibs-foundation has a slightly more expansive contract than other portions of the library; while the rest need to be _consistent_ with the behavior of the Darwin Foundation library, but not necessarily identical, `NSCoding` implementations in s-c-f must as far as possible be able to both decode Darwin Foundation archives and encode archives that Darwin Foundation can decode. Thus, simple roundtrip tests aren't sufficient. |
| 199 | + |
| 200 | +We have an infrastructure in place for this kind of test. We produce values (called _fixtures_) from closures specified in the file `FixtureValues.swift`. We can both test for in-memory roundtrips (ensuring that the data produced by swift-corelibs-foundation is also decodable with swift-corelibs-foundation), and run those closures using Darwin Foundation to produce archives that we then try to read with swift-corelibs-foundation (and, in the future, expand this to multiple sources). |
| 201 | + |
| 202 | +If you want to add a fixture to test, follow these steps: |
| 203 | + |
| 204 | +* Add the fixture or fixtures as static properties on the Fixture enum. For example: |
| 205 | + |
| 206 | +```swift |
| 207 | +static let defaultBeverage = TypedFixture<NSBeverage>("NSBeverage-Default") { |
| 208 | + return NSBeverage() |
| 209 | +} |
| 210 | + |
| 211 | +static let fancyBeverage = TypedFixture<NSBeverage>("NSBeverage-Fancy") { |
| 212 | + var options: NSBeverage.Options = .defaultForFancyDrinks |
| 213 | + options.insert(.shaken) |
| 214 | + options.remove(.stirren) |
| 215 | + return NSBeverage(named: "The Fancy Brand", options: options) |
| 216 | +} |
| 217 | +``` |
| 218 | + |
| 219 | +The string you pass to the constructor is an identifier for that particular fixture kind, and is used as the filename for the archive you will produce below. |
| 220 | + |
| 221 | +* Add them to the `_listOfAllFixtures` in the same file, wrapping them in the type eraser `AnyFixture`: |
| 222 | + |
| 223 | +```swift |
| 224 | +// Search for this: |
| 225 | +static let _listOfAllFixtures: [AnyFixture] = [ |
| 226 | + … |
| 227 | + // And insert them here: |
| 228 | + AnyFixture(Fixtures.defaultBeverage), |
| 229 | + AnyFixture(Fixtures.fancyBeverage), |
| 230 | +] |
| 231 | +``` |
| 232 | + |
| 233 | +* Add tests to the appropriate class that invoke the `assertValueRoundtripsInCoder` and `assertLoadedValuesMatch` methods. For example: |
| 234 | + |
| 235 | +```swift |
| 236 | +class TestNSBeverage { |
| 237 | + … |
| 238 | + |
| 239 | + let fixtures = [ |
| 240 | + Fixtures.defaultBeverage, |
| 241 | + Fixtures.fancyBeverage, |
| 242 | + ] |
| 243 | + |
| 244 | + func testCodingRoundtrip() throws { |
| 245 | + for fixture in fixtures { |
| 246 | + try fixture.assertValueRoundtripsInCoder() |
| 247 | + } |
| 248 | + } |
| 249 | + |
| 250 | + func testLoadingFixtures() throws { |
| 251 | + for fixture in fixtures { |
| 252 | + try fixture.assertLoadedValuesMatch() |
| 253 | + } |
| 254 | + } |
| 255 | + |
| 256 | + // Make sure the tests above are added to allTests, as usual! |
| 257 | +} |
| 258 | +``` |
| 259 | + |
| 260 | +These calls assume your objects override `isEqual(_:)` to be something other than object identity, and that it will return `true` for comparing the freshly-decoded objects to their originals. If that's not the case, you'll have to write a function that compares the old and new object for your use case: |
| 261 | + |
| 262 | +```swift |
| 263 | + func areEqual(_ lhs: NSBeverage, _ rhs: NSBeverage) -> Bool { |
| 264 | + return lhs.name.caseInsensitiveCompare(rhs.name) == .orderedSame && |
| 265 | + lhs.options == rhs.options |
| 266 | + } |
| 267 | + |
| 268 | + func testCodingRoundtrip() throws { |
| 269 | + for fixture in fixtures { |
| 270 | + try fixture.assertValueRoundtripsInCoder(matchingWith: areEqual(_:_:)) |
| 271 | + } |
| 272 | + } |
| 273 | + |
| 274 | + func testLoadingFixtures() throws { |
| 275 | + for fixture in fixtures { |
| 276 | + try fixture.assertLoadedValuesMatch(areEqual(_:_:)) |
| 277 | + } |
| 278 | + } |
| 279 | +``` |
| 280 | + |
| 281 | +* Open the `GenerateTestFixtures` project from the `Tools/GenerateTestFixtures` directory of the repository, and build the executable the eponymous target produces; then, run it. It will produce archives for all fixtures, and print their paths to the console. **Please run this step on the latest released version of macOS.** Do not run this step on newer beta versions, even if they're available. For example, at the time of writing, the most recently released version of macOS is macOS Mojave (10.14), and beta versions of macOS Catalina (10.15) are available; you'd need to run this step on a Mac running macOS Mojave. |
| 282 | + |
| 283 | +* Copy the new archives for your data from the location printed in the console to the `TestFoundation/Fixtures` directory of the repository. This will allow `assertLoadedValuesMatch` to find them. |
| 284 | + |
| 285 | +* Run your new tests and make sure they pass! |
| 286 | + |
| 287 | +The archive will be encoded using secure coding, if your class conforms to it and returns `true` from `supportsSecureCoding`. |
0 commit comments