From 4ff1a44e4a6e4e304892167ab0518a4c2d04f677 Mon Sep 17 00:00:00 2001 From: Paul Taykalo Date: Thu, 26 Jun 2025 17:08:01 +0300 Subject: [PATCH 1/2] Remove deduplication and simplify test setup --- ...DependenciesResolverRunnerUnzipTests.swift | 61 +++++++------------ 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift b/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift index 6e83ded..ee3fac0 100644 --- a/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift +++ b/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift @@ -4,7 +4,7 @@ import Foundation import Utils @Suite("DependenciesResolverRunner Unzip Method Tests") -struct DependenciesResolverRunnerUnzipTests { +final class DependenciesResolverRunnerUnzipTests { let sampleDependency: Dependency = Dependency( repo: "org/repo", tag: "1.0.0", @@ -22,6 +22,11 @@ struct DependenciesResolverRunnerUnzipTests { FileManager.default.temporaryDirectory .appending(pathComponents: "binary-dependency-manager-tests", UUID().uuidString, isDirectory: true) }() + + lazy var sampleAsset = sampleDependency.assets[0] + lazy var fileManager = FileManagerProtocolMock(tempDir: tempDir) + lazy var unarchiverMock = UnarchiverProtocolMock() + lazy var runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) func makeRunner( fileManager: FileManagerProtocolMock = FileManagerProtocolMock(), @@ -39,8 +44,6 @@ struct DependenciesResolverRunnerUnzipTests { } func setupFileManagerForUnzip( - _ fileManager: FileManagerProtocolMock, - dependency: Dependency, asset: Dependency.Asset, tempContents: [String] = ["Library.framework", "Info.plist"] ) { @@ -56,10 +59,6 @@ struct DependenciesResolverRunnerUnzipTests { @Test("unzip skips when shouldResolve returns false") func unzip_skipsWhenShouldResolveReturnsFalse() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] // Set up hash file to make shouldResolve return false let hashURL = try runner.outputDirectoryHashFile(for: sampleDependency, asset: sampleAsset) @@ -76,12 +75,9 @@ struct DependenciesResolverRunnerUnzipTests { @Test("unzip creates temp directory and calls unarchiver") func unzip_createsDirectoryAndCallsUnarchiver() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] + - setupFileManagerForUnzip(fileManager, dependency: sampleDependency, asset: sampleAsset) + setupFileManagerForUnzip(asset: sampleAsset) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) @@ -102,13 +98,10 @@ struct DependenciesResolverRunnerUnzipTests { @Test("unzip copies files from temp to output directory") func unzip_copiesFilesToOutput() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] + let tempContents = ["Library.framework", "Info.plist"] - setupFileManagerForUnzip(fileManager, dependency: sampleDependency, asset: sampleAsset, tempContents: tempContents) + setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) @@ -130,13 +123,10 @@ struct DependenciesResolverRunnerUnzipTests { @Test("unzip removes existing files before copying") func unzip_removesExistingFiles() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] + let tempContents = ["Library.framework"] - setupFileManagerForUnzip(fileManager, dependency: sampleDependency, asset: sampleAsset, tempContents: tempContents) + setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) // Set up existing file in output directory let outputDir = runner.outputDirectoryURL(for: sampleDependency, asset: sampleAsset) @@ -154,12 +144,7 @@ struct DependenciesResolverRunnerUnzipTests { @Test("unzip cleans up temporary directory") func unzip_cleansUpTempDirectory() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] - - setupFileManagerForUnzip(fileManager, dependency: sampleDependency, asset: sampleAsset) + setupFileManagerForUnzip(asset: sampleAsset) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) @@ -184,9 +169,7 @@ struct DependenciesResolverRunnerUnzipTests { tag: "1.0.0", assets: [assetWithoutContents] ) - - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() + let runner = makeRunner( fileManager: fileManager, dependencies: [dependencyWithoutContents], @@ -212,14 +195,15 @@ struct DependenciesResolverRunnerUnzipTests { let destURL1 = outputDir.appending(pathComponents: "file1.txt", isDirectory: false) #expect(fileManager.copiedFiles[sourceURL1] == destURL1) } + + func stubUnarchiverError(_ error: Error) { + self.unarchiverMock = UnarchiverProtocolMock(errorToThrow: error) + } @Test("unzip propagates unarchiver errors") func unzip_propagatesUnarchiverErrors() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock(errorToThrow: GenericError("Unzip failed")) - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] + stubUnarchiverError(GenericError("Unzip failed")) // WHEN & THEN #expect(throws: Error.self) { @@ -237,13 +221,10 @@ struct DependenciesResolverRunnerUnzipTests { @Test("unzip removes temporary files after copying") func unzip_removesTemporaryFilesAfterCopying() async throws { // GIVEN - let fileManager = FileManagerProtocolMock(tempDir: tempDir) - let unarchiverMock = UnarchiverProtocolMock() - let runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) - let sampleAsset = sampleDependency.assets[0] + let tempContents = ["Library.framework"] - setupFileManagerForUnzip(fileManager, dependency: sampleDependency, asset: sampleAsset, tempContents: tempContents) + setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) From 8bc9f6c8ecbc0ae4dc48a244d4aa87adbf654c4e Mon Sep 17 00:00:00 2001 From: Paul Taykalo Date: Thu, 26 Jun 2025 17:23:17 +0300 Subject: [PATCH 2/2] A bit refactored tests --- .../DepeneciesResolverRunner.swift | 11 ++++++-- ...DependenciesResolverRunnerUnzipTests.swift | 27 ++++++++++--------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/Sources/BinaryDependencyManager/DepeneciesResolverRunner.swift b/Sources/BinaryDependencyManager/DepeneciesResolverRunner.swift index 6e1b1c5..e055880 100644 --- a/Sources/BinaryDependencyManager/DepeneciesResolverRunner.swift +++ b/Sources/BinaryDependencyManager/DepeneciesResolverRunner.swift @@ -121,7 +121,7 @@ public struct DependenciesResolverRunner { func unzip(_ dependency: Dependency, asset: Dependency.Asset) throws { guard try shouldResolve(dependency, asset: asset) else { return } - let tempRootDirURL = fileManager.temporaryDirectory.appending(pathComponents: "PrivateDownloads", isDirectory: true) + let tempRootDirURL = fileManager.privateDownloadsDirectoryURL let tempDir = tempRootDirURL.appending(pathComponents: uuidString, isDirectory: true) try createDirectoryIfNeeded(at: tempDir) @@ -160,7 +160,7 @@ public struct DependenciesResolverRunner { Logger.log("[Unzip] Successfully unzipped \(dependency.repo) to \(outputDirectory.relativeFilePath)") } - + func isFileDownloaded(for dependency: Dependency, asset: Dependency.Asset) throws -> Bool { let downloadedFileURL = downloadURL(for: dependency, asset: asset) guard fileManager.fileExists(at: downloadedFileURL) else { return false } @@ -237,3 +237,10 @@ public struct DependenciesResolverRunner { try checksumCalculator.calculateChecksum(fileURL: fileURL) } } + + +extension FileManagerProtocol { + var privateDownloadsDirectoryURL: URL { + temporaryDirectory.appending(pathComponents: "PrivateDownloads", isDirectory: true) + } +} diff --git a/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift b/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift index ee3fac0..2328e24 100644 --- a/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift +++ b/Tests/BinaryDependencyManagerTests/DependenciesResolverRunner/DependenciesResolverRunnerUnzipTests.swift @@ -27,6 +27,7 @@ final class DependenciesResolverRunnerUnzipTests { lazy var fileManager = FileManagerProtocolMock(tempDir: tempDir) lazy var unarchiverMock = UnarchiverProtocolMock() lazy var runner = makeRunner(fileManager: fileManager, unarchiverMock: unarchiverMock) + func makeRunner( fileManager: FileManagerProtocolMock = FileManagerProtocolMock(), @@ -43,17 +44,22 @@ final class DependenciesResolverRunnerUnzipTests { ) } + @discardableResult func setupFileManagerForUnzip( asset: Dependency.Asset, tempContents: [String] = ["Library.framework", "Info.plist"] - ) { + ) -> URL { + let privateURLs = fileManager.privateDownloadsDirectoryURL + let unzipingDirectory: URL = privateURLs.appending(pathComponents: "mock-uuid", isDirectory: true) + // Make shouldResolve return true (no hash file exists) // This is the default behavior when hash file doesn't exist // Set up temp directory contents after unzipping fileManager.directoryContents = [ - tempDir.appending(pathComponents: "PrivateDownloads", "mock-uuid", asset.contents ?? "", isDirectory: true).filePath: tempContents + unzipingDirectory.appending(pathComponents: asset.contents ?? "", isDirectory: true).filePath: tempContents ] + return unzipingDirectory } @Test("unzip skips when shouldResolve returns false") @@ -75,17 +81,14 @@ final class DependenciesResolverRunnerUnzipTests { @Test("unzip creates temp directory and calls unarchiver") func unzip_createsDirectoryAndCallsUnarchiver() async throws { // GIVEN - - - setupFileManagerForUnzip(asset: sampleAsset) + let unzippingDirectory = setupFileManagerForUnzip(asset: sampleAsset) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) // THEN // Verify temp directory was created - let tempDir = fileManager.temporaryDirectory.appending(pathComponents: "PrivateDownloads", "mock-uuid", isDirectory: true) - #expect(fileManager.createdDirectories.contains(tempDir)) + #expect(fileManager.createdDirectories.contains(unzippingDirectory)) // Verify unarchiver was called with correct parameters #expect(unarchiverMock.unzipCalls.count == 1) @@ -101,7 +104,7 @@ final class DependenciesResolverRunnerUnzipTests { let tempContents = ["Library.framework", "Info.plist"] - setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) + let unzippingDirectory = setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) @@ -112,7 +115,7 @@ final class DependenciesResolverRunnerUnzipTests { #expect(fileManager.createdDirectories.contains(outputDir)) // Verify files were copied - let contentsDir = fileManager.temporaryDirectory.appending(pathComponents: "PrivateDownloads", "mock-uuid", sampleAsset.contents ?? "", isDirectory: true) + let contentsDir = unzippingDirectory.appending(pathComponents: sampleAsset.contents ?? "", isDirectory: true) for item in tempContents { let sourceURL = contentsDir.appending(pathComponents: item, isDirectory: false) let destinationURL = outputDir.appending(pathComponents: item, isDirectory: false) @@ -151,7 +154,7 @@ final class DependenciesResolverRunnerUnzipTests { // THEN // Verify temp root directory was removed - let tempRootDir = fileManager.temporaryDirectory.appending(pathComponents: "PrivateDownloads", isDirectory: true) + let tempRootDir = fileManager.privateDownloadsDirectoryURL #expect(fileManager.removedItems.contains(tempRootDir)) } @@ -224,14 +227,14 @@ final class DependenciesResolverRunnerUnzipTests { let tempContents = ["Library.framework"] - setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) + let unzpipingDirectory = setupFileManagerForUnzip(asset: sampleAsset, tempContents: tempContents) // WHEN try runner.unzip(sampleDependency, asset: sampleAsset) // THEN // Verify temporary files were removed after copying - let contentsDir = fileManager.temporaryDirectory.appending(pathComponents: "PrivateDownloads", "mock-uuid", sampleAsset.contents ?? "", isDirectory: true) + let contentsDir = unzpipingDirectory.appending(pathComponents: sampleAsset.contents ?? "", isDirectory: true) let tempFileURL = contentsDir.appending(pathComponents: "Library.framework", isDirectory: false) #expect(fileManager.removedItems.contains(tempFileURL)) }