From ad1809d3c60b783f4b9921da2917145265a82736 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Sep 2023 16:03:16 -0700 Subject: [PATCH 1/2] [Tests] Use `try` instead of `try!` in throwable context Avoid `try!` that can crash the tests unnecessarily in a throwable context that can just handle the error. --- .../ExplicitModuleBuildTests.swift | 26 +++++++++---------- .../IncrementalBuildPerformanceTests.swift | 2 +- .../IncrementalCompilationTests.swift | 6 ++--- Tests/SwiftDriverTests/SwiftDriverTests.swift | 6 ++--- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift index 35fa6c04f..faeb2b602 100644 --- a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift +++ b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift @@ -932,9 +932,9 @@ final class ExplicitModuleBuildTests: XCTestCase { main.nativePathString(escaped: true)] + sdkArgumentsForTesting let deps = - try! dependencyOracle.getImports(workingDirectory: path, - moduleAliases: ["Car": "Bar"], - commandLine: scannerCommand) + try dependencyOracle.getImports(workingDirectory: path, + moduleAliases: ["Car": "Bar"], + commandLine: scannerCommand) XCTAssertTrue(deps.imports.contains("Bar")) XCTAssertFalse(deps.imports.contains("Car")) @@ -1192,8 +1192,8 @@ final class ExplicitModuleBuildTests: XCTestCase { scannerCommand.removeFirst() } let dependencyGraph = - try! dependencyOracle.getDependencies(workingDirectory: path, - commandLine: scannerCommand) + try dependencyOracle.getDependencies(workingDirectory: path, + commandLine: scannerCommand) let fooDependencyInfo = try XCTUnwrap(dependencyGraph.modules[.swiftPrebuiltExternal("Foo")]) guard case .swiftPrebuiltExternal(let fooDetails) = fooDependencyInfo.details else { @@ -1299,8 +1299,8 @@ final class ExplicitModuleBuildTests: XCTestCase { main.nativePathString(escaped: true)] + sdkArgumentsForTesting let imports = - try! dependencyOracle.getImports(workingDirectory: path, - commandLine: scannerCommand) + try dependencyOracle.getImports(workingDirectory: path, + commandLine: scannerCommand) let expectedImports = ["C", "E", "G", "Swift", "SwiftOnoneSupport"] // Dependnig on how recent the platform we are running on, the _Concurrency module may or may not be present. let expectedImports2 = ["C", "E", "G", "Swift", "SwiftOnoneSupport", "_Concurrency"] @@ -1414,9 +1414,9 @@ final class ExplicitModuleBuildTests: XCTestCase { scannerCommand.removeFirst() } let _ = - try! dependencyOracle.getDependencies(workingDirectory: path, - commandLine: scannerCommand) - let potentialDiags = try! dependencyOracle.getScannerDiagnostics() + try dependencyOracle.getDependencies(workingDirectory: path, + commandLine: scannerCommand) + let potentialDiags = try dependencyOracle.getScannerDiagnostics() XCTAssertEqual(potentialDiags?.count, 5) let diags = try XCTUnwrap(potentialDiags) let error = diags[0] @@ -1748,7 +1748,7 @@ final class ExplicitModuleBuildTests: XCTestCase { } let firstScanGraph = - try! firstDependencyOracle.getDependencies(workingDirectory: path, + try firstDependencyOracle.getDependencies(workingDirectory: path, commandLine: scannerCommand) firstDependencyOracle.serializeScannerCache(to: cacheSavePath) @@ -1762,8 +1762,8 @@ final class ExplicitModuleBuildTests: XCTestCase { } XCTAssertFalse(secondDependencyOracle.loadScannerCache(from: cacheSavePath)) let secondScanGraph = - try! secondDependencyOracle.getDependencies(workingDirectory: path, - commandLine: scannerCommand) + try secondDependencyOracle.getDependencies(workingDirectory: path, + commandLine: scannerCommand) XCTAssertTrue(firstScanGraph.modules.count == secondScanGraph.modules.count) } diff --git a/Tests/SwiftDriverTests/IncrementalBuildPerformanceTests.swift b/Tests/SwiftDriverTests/IncrementalBuildPerformanceTests.swift index 38c4f5f54..d1811369a 100644 --- a/Tests/SwiftDriverTests/IncrementalBuildPerformanceTests.swift +++ b/Tests/SwiftDriverTests/IncrementalBuildPerformanceTests.swift @@ -101,7 +101,7 @@ class IncrementalBuildPerformanceTests: XCTestCase { ) throws -> (OutputFileMap, [SwiftSourceFile]) { let workingDirectory = localFileSystem.currentWorkingDirectory! let swiftDepsDirPath = try VirtualPath.init(path: swiftDepsDirectory).resolvedRelativePath(base: workingDirectory).absolutePath! - let withoutExtensions: ArraySlice = try! localFileSystem.getDirectoryContents(swiftDepsDirPath) + let withoutExtensions: ArraySlice = try localFileSystem.getDirectoryContents(swiftDepsDirPath) .compactMap { fileName -> Substring? in guard let suffixRange = fileName.range(of: ".swiftdeps"), diff --git a/Tests/SwiftDriverTests/IncrementalCompilationTests.swift b/Tests/SwiftDriverTests/IncrementalCompilationTests.swift index dde19e747..04a94832f 100644 --- a/Tests/SwiftDriverTests/IncrementalCompilationTests.swift +++ b/Tests/SwiftDriverTests/IncrementalCompilationTests.swift @@ -151,12 +151,12 @@ extension IncrementalCompilationTests { env["SWIFT_DRIVER_SWIFT_AUTOLINK_EXTRACT_EXEC"] = "/garbage/swift-autolink-extract" env["SWIFT_DRIVER_DSYMUTIL_EXEC"] = "/garbage/dsymutil" - var driver = try! Driver( + var driver = try Driver( args: commonArgs + ["-emit-library", "-target", "x86_64-unknown-linux"], env: env) - let plannedJobs = try! driver.planBuild() - let autolinkExtractJob = try! XCTUnwrap( + let plannedJobs = try driver.planBuild() + let autolinkExtractJob = try XCTUnwrap( plannedJobs .filter { $0.kind == .autolinkExtract } .first) diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index fdad0e438..c18255b5b 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -1422,7 +1422,7 @@ final class SwiftDriverTests: XCTestCase { guard let preserveCwd = localFileSystem.currentWorkingDirectory else { fatalError() } - try! localFileSystem.changeCurrentWorkingDirectory(to: path) + try localFileSystem.changeCurrentWorkingDirectory(to: path) defer { try! localFileSystem.changeCurrentWorkingDirectory(to: preserveCwd) } let diags = DiagnosticsEngine() @@ -1451,7 +1451,7 @@ final class SwiftDriverTests: XCTestCase { guard let preserveCwd = localFileSystem.currentWorkingDirectory else { fatalError() } - try! localFileSystem.changeCurrentWorkingDirectory(to: path) + try localFileSystem.changeCurrentWorkingDirectory(to: path) defer { try! localFileSystem.changeCurrentWorkingDirectory(to: preserveCwd) } try localFileSystem.createDirectory(path.appending(component: "subdir")) @@ -5151,7 +5151,7 @@ final class SwiftDriverTests: XCTestCase { let root = localFileSystem.currentWorkingDirectory!.appending(components: "build") let errorOutputFile = path.appending(component: "dummy_error_stream") - TSCBasic.stderrStream = try! ThreadSafeOutputByteStream(LocalFileOutputByteStream(errorOutputFile)) + TSCBasic.stderrStream = try ThreadSafeOutputByteStream(LocalFileOutputByteStream(errorOutputFile)) let libObj: AbsolutePath = root.appending(component: "lib.o") let mainObj: AbsolutePath = root.appending(component: "main.o") From feeec175c4ba61e579c98081c21fc6e7f8281d04 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Sep 2023 16:40:26 -0700 Subject: [PATCH 2/2] [Test] remove temp directory in `tearDown` instead of `deinit` Testcase can be deinit without calling `setUp` in case of something fatal happens. This will cause unwrapping nil during tests if running tests with `--parallel`. --- Tests/SwiftDriverTests/SwiftDriverTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SwiftDriverTests/SwiftDriverTests.swift b/Tests/SwiftDriverTests/SwiftDriverTests.swift index c18255b5b..2ee1b9b8d 100644 --- a/Tests/SwiftDriverTests/SwiftDriverTests.swift +++ b/Tests/SwiftDriverTests/SwiftDriverTests.swift @@ -61,7 +61,7 @@ final class SwiftDriverTests: XCTestCase { } } - deinit { + override func tearDown() { try? localFileSystem.removeFileTree(AbsolutePath(validating: self.ld.dirname)) }