From a7a07a241a32b351436b4e2480a18df9d740ad20 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sun, 25 Jun 2023 11:45:51 -0400 Subject: [PATCH 1/7] Add @MainActor to waitForExpectations(timeout:handler:) #428 --- .../XCTestCase+Asynchronous.swift | 2 +- Sources/XCTest/Public/XCTestCase.swift | 2 +- .../Asynchronous/Expectations/main.swift | 43 +++++++++++++++++-- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/Sources/XCTest/Public/Asynchronous/XCTestCase+Asynchronous.swift b/Sources/XCTest/Public/Asynchronous/XCTestCase+Asynchronous.swift index 7b122532..b9935ff7 100644 --- a/Sources/XCTest/Public/Asynchronous/XCTestCase+Asynchronous.swift +++ b/Sources/XCTest/Public/Asynchronous/XCTestCase+Asynchronous.swift @@ -41,7 +41,7 @@ public extension XCTestCase { /// these environments. To ensure compatibility of tests between /// swift-corelibs-xctest and Apple XCTest, it is not recommended to pass /// explicit values for `file` and `line`. - // FIXME: This should have `@MainActor` to match Xcode XCTest, but adding it causes errors in tests authored pre-Swift Concurrency which don't typically have `@MainActor`. + @preconcurrency @MainActor func waitForExpectations(timeout: TimeInterval, file: StaticString = #file, line: Int = #line, handler: XCWaitCompletionHandler? = nil) { precondition(Thread.isMainThread, "\(#function) must be called on the main thread") if currentWaiter != nil { diff --git a/Sources/XCTest/Public/XCTestCase.swift b/Sources/XCTest/Public/XCTestCase.swift index aadfecc7..4d734cd8 100644 --- a/Sources/XCTest/Public/XCTestCase.swift +++ b/Sources/XCTest/Public/XCTestCase.swift @@ -48,7 +48,7 @@ open class XCTestCase: XCTest { return 1 } - // FIXME: Once `waitForExpectations(timeout:...handler:)` gains `@MainActor`, this may be able to add it as well. + @MainActor internal var currentWaiter: XCTWaiter? /// The set of expectations made upon this test case. diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index 6ebb3352..f3566da0 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -551,7 +551,37 @@ class ExpectationsTestCase: XCTestCase { RunLoop.main.run(until: Date() + 1) } - static var allTests = { +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' failed \(\d+\.\d+ seconds\) + func test_waitForExpectationsAsync() async { + // Basic check that waitForExpectations() is functional when used with the + // await keyword in an async function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + await self.waitForExpectations(timeout: 0.0) + } + +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' failed \(\d+\.\d+ seconds\) + @MainActor func test_waitForExpectationsFromMainActor() { + // Basic check that waitForExpectations() is functional and does not need + // the await keyword when used from a main-actor-isolated test function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + self.waitForExpectations(timeout: 0.0) + } + +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' failed \(\d+\.\d+ seconds\) + @MainActor func test_waitForExpectationsFromMainActor_async() async { + // Basic check that waitForExpectations() is functional and does not need + // the await keyword when used from a main-actor-isolated test function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + self.waitForExpectations(timeout: 0.0) + } + + static var allTests: [(String, (ExpectationsTestCase) -> () throws -> Void)] = { return [ ("test_waitingForAnUnfulfilledExpectation_fails", test_waitingForAnUnfulfilledExpectation_fails), ("test_waitingForUnfulfilledExpectations_outputsAllExpectations_andFails", test_waitingForUnfulfilledExpectations_outputsAllExpectations_andFails), @@ -603,15 +633,20 @@ class ExpectationsTestCase: XCTestCase { ("test_expectationCreationOnSecondaryThread", test_expectationCreationOnSecondaryThread), ("test_expectationCreationWhileWaiting", test_expectationCreationWhileWaiting), ("test_runLoopInsideDispatch", test_runLoopInsideDispatch), + + // waitForExpectations() + @MainActor + ("test_waitForExpectationsAsync", asyncTest(test_waitForExpectationsAsync)), + ("test_waitForExpectationsFromMainActor", asyncTest { test_waitForExpectationsFromMainActor($0) }), + ("test_waitForExpectationsFromMainActor_async", asyncTest { test_waitForExpectationsFromMainActor_async($0) }), ] }() } // CHECK: Test Suite 'ExpectationsTestCase' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 35 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 38 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds XCTMain([testCase(ExpectationsTestCase.allTests)]) // CHECK: Test Suite '.*\.xctest' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 35 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 38 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds // CHECK: Test Suite 'All tests' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 35 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 38 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds From 5607291b60f5af473337a0b682b45cbfa5e374e5 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 26 Jun 2023 14:33:29 -0400 Subject: [PATCH 2/7] Fix typo in CHECK --- Tests/Functional/Asynchronous/Expectations/main.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index f3566da0..6c1cb8b9 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -551,8 +551,8 @@ class ExpectationsTestCase: XCTestCase { RunLoop.main.run(until: Date() + 1) } -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' failed \(\d+\.\d+ seconds\) +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' failed \(\d+\.\d+ seconds\) func test_waitForExpectationsAsync() async { // Basic check that waitForExpectations() is functional when used with the // await keyword in an async function. From 33f65a582db36447bd9a7120a4ec849079e5a15a Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 26 Jun 2023 14:52:46 -0400 Subject: [PATCH 3/7] Fix whitespace and try asyncTest() directly again to see if the compiler is happier --- .../Asynchronous/Expectations/main.swift | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index 6c1cb8b9..19ecb73b 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -554,34 +554,34 @@ class ExpectationsTestCase: XCTestCase { // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' failed \(\d+\.\d+ seconds\) func test_waitForExpectationsAsync() async { - // Basic check that waitForExpectations() is functional when used with the - // await keyword in an async function. - let expectation = self.expectation(description: "foo") - expectation.fulfill() - await self.waitForExpectations(timeout: 0.0) + // Basic check that waitForExpectations() is functional when used with the + // await keyword in an async function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + await self.waitForExpectations(timeout: 0.0) } // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' failed \(\d+\.\d+ seconds\) @MainActor func test_waitForExpectationsFromMainActor() { - // Basic check that waitForExpectations() is functional and does not need - // the await keyword when used from a main-actor-isolated test function. - let expectation = self.expectation(description: "foo") - expectation.fulfill() - self.waitForExpectations(timeout: 0.0) + // Basic check that waitForExpectations() is functional and does not need + // the await keyword when used from a main-actor-isolated test function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + self.waitForExpectations(timeout: 0.0) } // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' failed \(\d+\.\d+ seconds\) @MainActor func test_waitForExpectationsFromMainActor_async() async { - // Basic check that waitForExpectations() is functional and does not need - // the await keyword when used from a main-actor-isolated test function. - let expectation = self.expectation(description: "foo") - expectation.fulfill() - self.waitForExpectations(timeout: 0.0) + // Basic check that waitForExpectations() is functional and does not need + // the await keyword when used from a main-actor-isolated test function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + self.waitForExpectations(timeout: 0.0) } - static var allTests: [(String, (ExpectationsTestCase) -> () throws -> Void)] = { + static var allTests = { return [ ("test_waitingForAnUnfulfilledExpectation_fails", test_waitingForAnUnfulfilledExpectation_fails), ("test_waitingForUnfulfilledExpectations_outputsAllExpectations_andFails", test_waitingForUnfulfilledExpectations_outputsAllExpectations_andFails), @@ -636,8 +636,8 @@ class ExpectationsTestCase: XCTestCase { // waitForExpectations() + @MainActor ("test_waitForExpectationsAsync", asyncTest(test_waitForExpectationsAsync)), - ("test_waitForExpectationsFromMainActor", asyncTest { test_waitForExpectationsFromMainActor($0) }), - ("test_waitForExpectationsFromMainActor_async", asyncTest { test_waitForExpectationsFromMainActor_async($0) }), + ("test_waitForExpectationsFromMainActor", asyncTest(test_waitForExpectationsFromMainActor)), + ("test_waitForExpectationsFromMainActor_async", asyncTest(test_waitForExpectationsFromMainActor_async)), ] }() } From 6f50082f8bc44cdce7871f78a549e7f9a9473456 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 26 Jun 2023 16:18:58 -0400 Subject: [PATCH 4/7] Disable @MainActor test functions due to SILGen crash. --- Tests/Functional/Asynchronous/Expectations/main.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index 19ecb73b..f023716b 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -636,17 +636,18 @@ class ExpectationsTestCase: XCTestCase { // waitForExpectations() + @MainActor ("test_waitForExpectationsAsync", asyncTest(test_waitForExpectationsAsync)), - ("test_waitForExpectationsFromMainActor", asyncTest(test_waitForExpectationsFromMainActor)), - ("test_waitForExpectationsFromMainActor_async", asyncTest(test_waitForExpectationsFromMainActor_async)), + // Disabled due to a SILGen crash: + // ("test_waitForExpectationsFromMainActor", asyncTest(test_waitForExpectationsFromMainActor)), + // ("test_waitForExpectationsFromMainActor_async", asyncTest(test_waitForExpectationsFromMainActor_async)), ] }() } // CHECK: Test Suite 'ExpectationsTestCase' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 38 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 36 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds XCTMain([testCase(ExpectationsTestCase.allTests)]) // CHECK: Test Suite '.*\.xctest' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 38 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 36 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds // CHECK: Test Suite 'All tests' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 38 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 36 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds From e889f206268fce3df9cd9886d62c50a99fc17d95 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 26 Jun 2023 17:25:33 -0400 Subject: [PATCH 5/7] Oops, wrong checks --- Tests/Functional/Asynchronous/Expectations/main.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index f023716b..285eb7f6 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -552,7 +552,7 @@ class ExpectationsTestCase: XCTestCase { } // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' failed \(\d+\.\d+ seconds\) +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsAsync' passed \(\d+\.\d+ seconds\) func test_waitForExpectationsAsync() async { // Basic check that waitForExpectations() is functional when used with the // await keyword in an async function. @@ -562,7 +562,7 @@ class ExpectationsTestCase: XCTestCase { } // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' failed \(\d+\.\d+ seconds\) +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' passed \(\d+\.\d+ seconds\) @MainActor func test_waitForExpectationsFromMainActor() { // Basic check that waitForExpectations() is functional and does not need // the await keyword when used from a main-actor-isolated test function. @@ -572,7 +572,7 @@ class ExpectationsTestCase: XCTestCase { } // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' failed \(\d+\.\d+ seconds\) +// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' passed \(\d+\.\d+ seconds\) @MainActor func test_waitForExpectationsFromMainActor_async() async { // Basic check that waitForExpectations() is functional and does not need // the await keyword when used from a main-actor-isolated test function. From a1bdcfe1514beb0814c5c893005a39e87872bed1 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 26 Jun 2023 18:41:35 -0400 Subject: [PATCH 6/7] Reenable one of the @MainActor tests and remove the other as redundant --- .../Asynchronous/Expectations/main.swift | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index 285eb7f6..bcf420d3 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -563,22 +563,14 @@ class ExpectationsTestCase: XCTestCase { // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ // CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor' passed \(\d+\.\d+ seconds\) - @MainActor func test_waitForExpectationsFromMainActor() { - // Basic check that waitForExpectations() is functional and does not need - // the await keyword when used from a main-actor-isolated test function. - let expectation = self.expectation(description: "foo") - expectation.fulfill() - self.waitForExpectations(timeout: 0.0) - } - -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' started at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: Test Case 'ExpectationsTestCase.test_waitForExpectationsFromMainActor_async' passed \(\d+\.\d+ seconds\) - @MainActor func test_waitForExpectationsFromMainActor_async() async { - // Basic check that waitForExpectations() is functional and does not need - // the await keyword when used from a main-actor-isolated test function. - let expectation = self.expectation(description: "foo") - expectation.fulfill() - self.waitForExpectations(timeout: 0.0) + func test_waitForExpectationsFromMainActor() async { + await MainActor.run { + // Basic check that waitForExpectations() is functional and does not need + // the await keyword when used from a main-actor-isolated test function. + let expectation = self.expectation(description: "foo") + expectation.fulfill() + self.waitForExpectations(timeout: 0.0) + } } static var allTests = { @@ -636,9 +628,7 @@ class ExpectationsTestCase: XCTestCase { // waitForExpectations() + @MainActor ("test_waitForExpectationsAsync", asyncTest(test_waitForExpectationsAsync)), - // Disabled due to a SILGen crash: - // ("test_waitForExpectationsFromMainActor", asyncTest(test_waitForExpectationsFromMainActor)), - // ("test_waitForExpectationsFromMainActor_async", asyncTest(test_waitForExpectationsFromMainActor_async)), + ("test_waitForExpectationsFromMainActor", asyncTest(test_waitForExpectationsFromMainActor)), ] }() } From 333c1eacc29989fa75176cf16deeefb8e25ec2a5 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 27 Jun 2023 09:49:11 -0400 Subject: [PATCH 7/7] Fix count in final checks. --- Tests/Functional/Asynchronous/Expectations/main.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Functional/Asynchronous/Expectations/main.swift b/Tests/Functional/Asynchronous/Expectations/main.swift index bcf420d3..8d0b1599 100644 --- a/Tests/Functional/Asynchronous/Expectations/main.swift +++ b/Tests/Functional/Asynchronous/Expectations/main.swift @@ -633,11 +633,11 @@ class ExpectationsTestCase: XCTestCase { }() } // CHECK: Test Suite 'ExpectationsTestCase' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 36 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 37 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds XCTMain([testCase(ExpectationsTestCase.allTests)]) // CHECK: Test Suite '.*\.xctest' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 36 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 37 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds // CHECK: Test Suite 'All tests' failed at \d+-\d+-\d+ \d+:\d+:\d+\.\d+ -// CHECK: \t Executed 36 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds +// CHECK: \t Executed 37 tests, with 16 failures \(2 unexpected\) in \d+\.\d+ \(\d+\.\d+\) seconds