From ce3d1c58ec772e5d75a7f5a94d137cdb5bfe5a3c Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Sun, 30 Apr 2023 12:39:26 -0700 Subject: [PATCH] Tests: prefer to use `first` rather than indexing In the case of a failure in the test, we would previously continue to perform the slice operation which will fail in the case of an Asserts runtime due to an out of bounds access. The use of `first?` here would unwrap or fail, allowing tests to continue execution. --- .../PD_4_2_LoadingTests.swift | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift b/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift index c1fecdaa157..9fcd6ff519e 100644 --- a/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift +++ b/Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift @@ -596,17 +596,17 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let manifestLoader = ManifestLoader(toolchain: try UserToolchain.default, cacheDir: path, delegate: delegate) - func check(loader: ManifestLoader, expectCached: Bool) { + func check(loader: ManifestLoader, expectCached: Bool) throws { delegate.clear() delegate.prepare(expectParsing: !expectCached) - let manifest = try! loader.load( + let manifest = try XCTUnwrap(loader.load( manifestPath: manifestPath, packageKind: .fileSystem(manifestPath.parentDirectory), toolsVersion: .v4_2, fileSystem: fs, observabilityScope: observability.topScope - ) + )) XCTAssertNoDiagnostics(observability.diagnostics) XCTAssertEqual(try delegate.loaded(timeout: .now() + 1), [manifestPath]) @@ -615,20 +615,20 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { XCTAssertEqual(manifest.targets[0].name, "foo") } - check(loader: manifestLoader, expectCached: false) - check(loader: manifestLoader, expectCached: true) + try check(loader: manifestLoader, expectCached: false) + try check(loader: manifestLoader, expectCached: true) try withCustomEnv(["SWIFTPM_MANIFEST_CACHE_TEST": "1"]) { - check(loader: manifestLoader, expectCached: false) - check(loader: manifestLoader, expectCached: true) + try check(loader: manifestLoader, expectCached: false) + try check(loader: manifestLoader, expectCached: true) } try withCustomEnv(["SWIFTPM_MANIFEST_CACHE_TEST": "2"]) { - check(loader: manifestLoader, expectCached: false) - check(loader: manifestLoader, expectCached: true) + try check(loader: manifestLoader, expectCached: false) + try check(loader: manifestLoader, expectCached: true) } - check(loader: manifestLoader, expectCached: true) + try check(loader: manifestLoader, expectCached: true) } } @@ -658,17 +658,17 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { let manifestLoader = ManifestLoader(toolchain: try UserToolchain.default, cacheDir: path, delegate: delegate) - func check(loader: ManifestLoader, expectCached: Bool) { + func check(loader: ManifestLoader, expectCached: Bool) throws { delegate.clear() delegate.prepare(expectParsing: !expectCached) - let manifest = try! loader.load( + let manifest = try XCTUnwrap(loader.load( manifestPath: manifestPath, packageKind: .fileSystem(manifestPath.parentDirectory), toolsVersion: .v4_2, fileSystem: fs, observabilityScope: observability.topScope - ) + )) XCTAssertNoDiagnostics(observability.diagnostics) XCTAssertEqual(try delegate.loaded(timeout: .now() + 1), [manifestPath]) @@ -677,9 +677,9 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { XCTAssertEqual(manifest.targets[0].name, "foo") } - check(loader: manifestLoader, expectCached: false) + try check(loader: manifestLoader, expectCached: false) for _ in 0..<2 { - check(loader: manifestLoader, expectCached: true) + try check(loader: manifestLoader, expectCached: true) } try fs.writeFileContents( @@ -700,14 +700,14 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests { """ ) - check(loader: manifestLoader, expectCached: false) + try check(loader: manifestLoader, expectCached: false) for _ in 0..<2 { - check(loader: manifestLoader, expectCached: true) + try check(loader: manifestLoader, expectCached: true) } let noCacheLoader = ManifestLoader(toolchain: try UserToolchain.default, delegate: delegate) for _ in 0..<2 { - check(loader: noCacheLoader, expectCached: false) + try check(loader: noCacheLoader, expectCached: false) } // Resetting the cache should allow us to remove the cache