From e782e156d33dba2a757be4d765df89c0ede97c63 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 29 Jan 2025 06:24:24 +0100 Subject: [PATCH 1/4] Add basic ShellClient --- .../App/Core/Dependencies/ShellClient.swift | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 Sources/App/Core/Dependencies/ShellClient.swift diff --git a/Sources/App/Core/Dependencies/ShellClient.swift b/Sources/App/Core/Dependencies/ShellClient.swift new file mode 100644 index 000000000..f607b3ed1 --- /dev/null +++ b/Sources/App/Core/Dependencies/ShellClient.swift @@ -0,0 +1,42 @@ +// Copyright Dave Verwer, Sven A. Schmidt, and other contributors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import Dependencies +import DependenciesMacros + + +@DependencyClient +struct ShellClient { + +} + + +extension ShellClient: DependencyKey { + static var liveValue: Self { + .init() + } +} + + +extension ShellClient: TestDependencyKey { + static var testValue: Self { .init() } +} + + +extension DependencyValues { + var shell: ShellClient { + get { self[ShellClient.self] } + set { self[ShellClient.self] = newValue } + } +} From fc32782ac05fa403d5066c778cac272081e3a363 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 29 Jan 2025 07:16:07 +0100 Subject: [PATCH 2/4] Move run to ShellClient --- Sources/App/Commands/Analyze.swift | 30 +- Sources/App/Core/AppEnvironment.swift | 21 +- .../App/Core/Dependencies/ShellClient.swift | 29 +- Sources/App/Core/Git.swift | 23 +- Tests/AppTests/AnalyzeErrorTests.swift | 14 +- Tests/AppTests/AnalyzerTests.swift | 398 +++++++++--------- Tests/AppTests/ErrorReportingTests.swift | 8 +- Tests/AppTests/GitTests.swift | 78 ++-- Tests/AppTests/IngestionTests.swift | 5 +- Tests/AppTests/MastodonTests.swift | 8 +- Tests/AppTests/Mocks/AppShell+mock.swift | 3 +- .../PackageController+routesTests.swift | 8 +- Tests/AppTests/PackageTests.swift | 6 +- Tests/AppTests/PipelineTests.swift | 6 +- Tests/AppTests/ReAnalyzeVersionsTests.swift | 71 ++-- 15 files changed, 362 insertions(+), 346 deletions(-) diff --git a/Sources/App/Commands/Analyze.swift b/Sources/App/Commands/Analyze.swift index f58be4bfb..05dd61cfe 100644 --- a/Sources/App/Commands/Analyze.swift +++ b/Sources/App/Commands/Analyze.swift @@ -256,8 +256,9 @@ extension Analyze { static func clone(cacheDir: String, url: String) async throws { Current.logger().info("cloning \(url) to \(cacheDir)") @Dependency(\.fileManager) var fileManager - try await Current.shell.run(command: .gitClone(url: URL(string: url)!, to: cacheDir), - at: fileManager.checkoutsDirectory()) + @Dependency(\.shell) var shell + try await shell.run(command: .gitClone(url: URL(string: url)!, to: cacheDir), + at: fileManager.checkoutsDirectory()) } @@ -269,22 +270,22 @@ extension Analyze { /// - Throws: Shell errors static func fetch(cacheDir: String, branch: String, url: String) async throws { @Dependency(\.fileManager) var fileManager + @Dependency(\.shell) var shell Current.logger().info("pulling \(url) in \(cacheDir)") // clean up stray lock files that might have remained from aborted commands for fileName in ["HEAD.lock", "index.lock"] { let filePath = cacheDir + "/.git/\(fileName)" if fileManager.fileExists(atPath: filePath) { Current.logger().info("Removing stale \(fileName) at path: \(filePath)") - try await Current.shell.run(command: .removeFile(from: filePath)) + try await shell.run(command: .removeFile(from: filePath), at: .cwd) } } // git reset --hard to deal with stray .DS_Store files on macOS - try await Current.shell.run(command: .gitReset(hard: true), at: cacheDir) - try await Current.shell.run(command: .gitClean, at: cacheDir) - try await Current.shell.run(command: .gitFetchAndPruneTags, at: cacheDir) - try await Current.shell.run(command: .gitCheckout(branch: branch), at: cacheDir) - try await Current.shell.run(command: .gitReset(to: branch, hard: true), - at: cacheDir) + try await shell.run(command: .gitReset(hard: true), at: cacheDir) + try await shell.run(command: .gitClean, at: cacheDir) + try await shell.run(command: .gitFetchAndPruneTags, at: cacheDir) + try await shell.run(command: .gitCheckout(branch: branch), at: cacheDir) + try await shell.run(command: .gitReset(to: branch, hard: true), at: cacheDir) } @@ -293,6 +294,8 @@ extension Analyze { /// - package: `Package` to refresh static func refreshCheckout(package: Joined) async throws { @Dependency(\.fileManager) var fileManager + @Dependency(\.shell) var shell + guard let cacheDir = fileManager.cacheDirectoryPath(for: package.model) else { throw AppError.invalidPackageCachePath(package.model.id, package.model.url) } @@ -311,7 +314,7 @@ extension Analyze { url: package.model.url) } catch { Current.logger().info("fetch failed: \(error.localizedDescription)") - try await Current.shell.run(command: .removeFile(from: cacheDir, arguments: ["-r", "-f"])) + try await shell.run(command: .removeFile(from: cacheDir, arguments: ["-r", "-f"]), at: .cwd) try await clone(cacheDir: cacheDir, url: package.model.url) } } catch { @@ -537,12 +540,13 @@ extension Analyze { /// - Returns: `Manifest` data static func dumpPackage(at path: String) async throws -> Manifest { @Dependency(\.fileManager) var fileManager + @Dependency(\.shell) var shell guard fileManager.fileExists(atPath: path + "/Package.swift") else { // It's important to check for Package.swift - otherwise `dump-package` will go // up the tree through parent directories to find one throw AppError.invalidRevision(nil, "no Package.swift") } - let json = try await Current.shell.run(command: .swiftDumpPackage, at: path) + let json = try await shell.run(command: .swiftDumpPackage, at: path) return try JSONDecoder().decode(Manifest.self, from: Data(json.utf8)) } @@ -561,12 +565,14 @@ extension Analyze { static func getPackageInfo(package: Joined, version: Version) async throws -> PackageInfo { // check out version in cache directory @Dependency(\.fileManager) var fileManager + @Dependency(\.shell) var shell + guard let cacheDir = fileManager.cacheDirectoryPath(for: package.model) else { throw AppError.invalidPackageCachePath(package.model.id, package.model.url) } - try await Current.shell.run(command: .gitCheckout(branch: version.reference.description), at: cacheDir) + try await shell.run(command: .gitCheckout(branch: version.reference.description), at: cacheDir) do { let packageManifest = try await dumpPackage(at: cacheDir) diff --git a/Sources/App/Core/AppEnvironment.swift b/Sources/App/Core/AppEnvironment.swift index 12d01edc2..5d8975266 100644 --- a/Sources/App/Core/AppEnvironment.swift +++ b/Sources/App/Core/AppEnvironment.swift @@ -41,28 +41,9 @@ extension AppEnvironment { +@available(*, deprecated) struct Shell: Sendable { - var run: @Sendable (ShellOutCommand, String) async throws -> String - - // also provide pass-through methods to preserve argument labels - @discardableResult - func run(command: ShellOutCommand, at path: String = ".") async throws -> String { - do { - return try await run(command, path) - } catch { - // re-package error to capture more information - throw AppError.shellCommandFailed(command.description, path, error.localizedDescription) - } - } - static let live: Self = .init( - run: { - let res = try await ShellOut.shellOut(to: $0, at: $1, logger: Current.logger()) - if !res.stderr.isEmpty { - Current.logger().warning("stderr: \(res.stderr)") - } - return res.stdout - } ) } diff --git a/Sources/App/Core/Dependencies/ShellClient.swift b/Sources/App/Core/Dependencies/ShellClient.swift index f607b3ed1..16c735055 100644 --- a/Sources/App/Core/Dependencies/ShellClient.swift +++ b/Sources/App/Core/Dependencies/ShellClient.swift @@ -14,17 +14,44 @@ import Dependencies import DependenciesMacros +import ShellOut @DependencyClient struct ShellClient { + var run: @Sendable (ShellOutCommand, String) async throws -> String +} + + +extension ShellClient { + @discardableResult + func run(command: ShellOutCommand, at path: String) async throws -> String { + try await run(command, path) + } +} + +extension String { + static let cwd = "." } extension ShellClient: DependencyKey { static var liveValue: Self { - .init() + .init( + run: { command, path in + do { + let res = try await ShellOut.shellOut(to: command, at: path, logger: Current.logger()) + if !res.stderr.isEmpty { + Current.logger().warning("stderr: \(res.stderr)") + } + return res.stdout + } catch { + // re-package error to capture more information + throw AppError.shellCommandFailed(command.description, path, error.localizedDescription) + } + } + ) } } diff --git a/Sources/App/Core/Git.swift b/Sources/App/Core/Git.swift index be1cb79d4..06ba30329 100644 --- a/Sources/App/Core/Git.swift +++ b/Sources/App/Core/Git.swift @@ -13,6 +13,8 @@ // limitations under the License. import Foundation + +import Dependencies import SemanticVersion import ShellOut @@ -26,7 +28,8 @@ enum Git { } static func commitCount(at path: String) async throws -> Int { - let res = try await Current.shell.run(command: .gitCommitCount, at: path) + @Dependency(\.shell) var shell + let res = try await shell.run(command: .gitCommitCount, at: path) guard let count = Int(res) else { throw Error.invalidInteger } @@ -34,8 +37,9 @@ enum Git { } static func firstCommitDate(at path: String) async throws -> Date { + @Dependency(\.shell) var shell let res = String( - try await Current.shell.run(command: .gitFirstCommitDate, at: path) + try await shell.run(command: .gitFirstCommitDate, at: path) .trimming { $0 == Character("\"") } ) guard let timestamp = TimeInterval(res) else { @@ -45,8 +49,9 @@ enum Git { } static func lastCommitDate(at path: String) async throws -> Date { + @Dependency(\.shell) var shell let res = String( - try await Current.shell.run(command: .gitLastCommitDate, at: path) + try await shell.run(command: .gitLastCommitDate, at: path) .trimming { $0 == Character("\"") } ) guard let timestamp = TimeInterval(res) else { @@ -56,7 +61,8 @@ enum Git { } static func getTags(at path: String) async throws -> [Reference] { - let tags = try await Current.shell.run(command: .gitListTags, at: path) + @Dependency(\.shell) var shell + let tags = try await shell.run(command: .gitListTags, at: path) return tags.split(separator: "\n") .map(String.init) .compactMap { tag in SemanticVersion(tag).map { ($0, tag) } } @@ -64,9 +70,10 @@ enum Git { } static func hasBranch(_ reference: Reference, at path: String) async throws -> Bool { + @Dependency(\.shell) var shell guard let branchName = reference.branchName else { return false } do { - _ = try await Current.shell.run(command: .gitHasBranch(branchName), at: path) + _ = try await shell.run(command: .gitHasBranch(branchName), at: path) return true } catch { return false @@ -74,9 +81,10 @@ enum Git { } static func revisionInfo(_ reference: Reference, at path: String) async throws -> RevisionInfo { + @Dependency(\.shell) var shell let separator = "-" let res = String( - try await Current.shell.run(command: .gitRevisionInfo(reference: reference, separator: separator), + try await shell.run(command: .gitRevisionInfo(reference: reference, separator: separator), at: path) .trimming { $0 == Character("\"") } ) @@ -92,7 +100,8 @@ enum Git { } static func shortlog(at path: String) async throws -> String { - try await Current.shell.run(command: .gitShortlog, at: path) + @Dependency(\.shell) var shell + return try await shell.run(command: .gitShortlog, at: path) } struct RevisionInfo: Equatable { diff --git a/Tests/AppTests/AnalyzeErrorTests.swift b/Tests/AppTests/AnalyzeErrorTests.swift index 73b622e06..07eacf3da 100644 --- a/Tests/AppTests/AnalyzeErrorTests.swift +++ b/Tests/AppTests/AnalyzeErrorTests.swift @@ -72,8 +72,6 @@ final class AnalyzeErrorTests: AppTestCase { Repository(package: pkgs[0], defaultBranch: "main", name: "1", owner: "foo"), Repository(package: pkgs[1], defaultBranch: "main", name: "2", owner: "foo"), ].save(on: app.db) - - Current.shell.run = Self.defaultShellRun } override func invokeTest() { @@ -106,6 +104,7 @@ final class AnalyzeErrorTests: AppTestCase { $0.httpClient.mastodonPost = { @Sendable [socialPosts = self.socialPosts] message in socialPosts.withValue { $0.append(message) } } + $0.shell.run = Self.defaultShellRun } operation: { super.invokeTest() } @@ -115,8 +114,7 @@ final class AnalyzeErrorTests: AppTestCase { try await withDependencies { $0.environment.loadSPIManifest = { _ in nil } $0.fileManager.fileExists = { @Sendable _ in true } - } operation: { - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in switch cmd { case _ where cmd.description.contains("git clone https://github.com/foo/1"): throw SimulatedError() @@ -128,7 +126,7 @@ final class AnalyzeErrorTests: AppTestCase { return try Self.defaultShellRun(cmd, path) } } - + } operation: { // MUT try await Analyze.analyze(client: app.client, database: app.db, mode: .limit(10)) @@ -172,9 +170,7 @@ final class AnalyzeErrorTests: AppTestCase { try await withDependencies { $0.environment.loadSPIManifest = { _ in nil } $0.fileManager.fileExists = { @Sendable _ in true } - } operation: { - // setup - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in switch cmd { case .gitCheckout(branch: "main", quiet: true) where path.hasSuffix("foo-1"): throw SimulatedError() @@ -183,7 +179,7 @@ final class AnalyzeErrorTests: AppTestCase { return try Self.defaultShellRun(cmd, path) } } - + } operation: { // MUT try await Analyze.analyze(client: app.client, database: app.db, mode: .limit(10)) diff --git a/Tests/AppTests/AnalyzerTests.swift b/Tests/AppTests/AnalyzerTests.swift index 55dae553f..c09a5e2d6 100644 --- a/Tests/AppTests/AnalyzerTests.swift +++ b/Tests/AppTests/AnalyzerTests.swift @@ -35,6 +35,7 @@ class AnalyzerTests: AppTestCase { // expected shell commands for the happy path.) let checkoutDir = QueueIsolated(nil) let firstDirCloned = QueueIsolated(false) + let commands = QueueIsolated<[Command]>([]) try await withDependencies { $0.date.now = .now $0.environment.allowSocialPosts = { true } @@ -58,26 +59,7 @@ class AnalyzerTests: AppTestCase { } $0.git = .liveValue $0.httpClient.mastodonPost = { @Sendable _ in } - } operation: { - // setup - let urls = ["https://github.com/foo/1", "https://github.com/foo/2"] - let pkgs = try await savePackages(on: app.db, urls.asURLs, processingStage: .ingestion) - try await Repository(package: pkgs[0], - defaultBranch: "main", - name: "1", - owner: "foo", - releases: [ - .mock(description: "rel 1.0.0", tagName: "1.0.0") - ], - stars: 25).save(on: app.db) - try await Repository(package: pkgs[1], - defaultBranch: "main", - name: "2", - owner: "foo", - stars: 100).save(on: app.db) - - let commands = QueueIsolated<[Command]>([]) - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in let trimmedPath = path.replacingOccurrences(of: checkoutDir.value!, with: ".") commands.withValue { $0.append(.init(command: cmd, path: trimmedPath)!) @@ -93,37 +75,37 @@ class AnalyzerTests: AppTestCase { } if cmd == .swiftDumpPackage && path.hasSuffix("foo-1") { return #""" - { - "name": "foo-1", - "products": [ { - "name": "p1", - "targets": ["t1"], - "type": { - "executable": null - } + "name": "foo-1", + "products": [ + { + "name": "p1", + "targets": ["t1"], + "type": { + "executable": null + } + } + ], + "targets": [{"name": "t1", "type": "executable"}] } - ], - "targets": [{"name": "t1", "type": "executable"}] - } - """# + """# } if cmd == .swiftDumpPackage && path.hasSuffix("foo-2") { return #""" - { - "name": "foo-2", - "products": [ { - "name": "p2", - "targets": ["t2"], - "type": { - "library": ["automatic"] - } + "name": "foo-2", + "products": [ + { + "name": "p2", + "targets": ["t2"], + "type": { + "library": ["automatic"] + } + } + ], + "targets": [{"name": "t2", "type": "regular"}] } - ], - "targets": [{"name": "t2", "type": "regular"}] - } - """# + """# } // Git.revisionInfo (per ref - default branch & tags) @@ -147,6 +129,23 @@ class AnalyzerTests: AppTestCase { return "" } + } operation: { + // setup + let urls = ["https://github.com/foo/1", "https://github.com/foo/2"] + let pkgs = try await savePackages(on: app.db, urls.asURLs, processingStage: .ingestion) + try await Repository(package: pkgs[0], + defaultBranch: "main", + name: "1", + owner: "foo", + releases: [ + .mock(description: "rel 1.0.0", tagName: "1.0.0") + ], + stars: 25).save(on: app.db) + try await Repository(package: pkgs[1], + defaultBranch: "main", + name: "2", + owner: "foo", + stars: 100).save(on: app.db) // MUT try await Analyze.analyze(client: app.client, @@ -248,8 +247,27 @@ class AnalyzerTests: AppTestCase { 2\tPerson 2 """ } - $0.httpClient.mastodonPost = { @Sendable _ in } + $0.shell.run = { @Sendable cmd, path in + if cmd.description.hasSuffix("package dump-package") { + return #""" + { + "name": "foo-1", + "products": [ + { + "name": "p1", + "targets": [], + "type": { + "executable": null + } + } + ], + "targets": [] + } + """# + } + return "" + } } operation: { // setup let pkgId = UUID() @@ -273,27 +291,6 @@ class AnalyzerTests: AppTestCase { packageName: "foo-1", reference: .tag(1, 0, 0)).save(on: app.db) - Current.shell.run = { @Sendable cmd, path in - if cmd.description.hasSuffix("package dump-package") { - return #""" - { - "name": "foo-1", - "products": [ - { - "name": "p1", - "targets": [], - "type": { - "executable": null - } - } - ], - "targets": [] - } - """# - } - return "" - } - // MUT try await Analyze.analyze(client: app.client, database: app.db, @@ -372,16 +369,7 @@ class AnalyzerTests: AppTestCase { 2\tPerson 2 """ } - } operation: { - // setup - let urls = ["https://github.com/foo/1", "https://github.com/foo/2"] - let pkgs = try await savePackages(on: app.db, urls.asURLs, processingStage: .ingestion) - for p in pkgs { - try await Repository(package: p, defaultBranch: "main").save(on: app.db) - } - let lastUpdate = Date() - - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in // first package fails if cmd.description.hasSuffix("swift package dump-package") && path.hasSuffix("foo-1") { return "bad data" @@ -392,6 +380,14 @@ class AnalyzerTests: AppTestCase { } return "" } + } operation: { + // setup + let urls = ["https://github.com/foo/1", "https://github.com/foo/2"] + let pkgs = try await savePackages(on: app.db, urls.asURLs, processingStage: .ingestion) + for p in pkgs { + try await Repository(package: p, defaultBranch: "main").save(on: app.db) + } + let lastUpdate = Date() // MUT try await Analyze.analyze(client: app.client, database: app.db, mode: .limit(10)) @@ -406,6 +402,24 @@ class AnalyzerTests: AppTestCase { func test_continue_on_exception() async throws { // Test to ensure exceptions don't interrupt processing let checkoutDir: NIOLockedValueBox = .init(nil) + let commands = QueueIsolated<[Command]>([]) + let refs: [Reference] = [.tag(1, 0, 0), .tag(1, 1, 1), .branch("main")] + let mockResults = { + var res: [ShellOutCommand: String] = [ + .gitListTags: refs.filter(\.isTag).map { "\($0)" }.joined(separator: "\n"), + .gitCommitCount: "12", + .gitFirstCommitDate: "0", + .gitLastCommitDate: "1", + .gitShortlog : """ + 10\tPerson 1 + 2\tPerson 2 + """ + ] + for (idx, ref) in refs.enumerated() { + res[.gitRevisionInfo(reference: ref)] = "sha-\(idx)" + } + return res + }() try await withDependencies { $0.date.now = .now $0.environment.allowSocialPosts = { true } @@ -418,34 +432,7 @@ class AnalyzerTests: AppTestCase { return false } $0.git = .liveValue - } operation: { - // setup - let urls = ["https://github.com/foo/1", "https://github.com/foo/2"] - let pkgs = try await savePackages(on: app.db, urls.asURLs, processingStage: .ingestion) - for p in pkgs { - try await Repository(package: p, defaultBranch: "main").save(on: app.db) - } - - let refs: [Reference] = [.tag(1, 0, 0), .tag(1, 1, 1), .branch("main")] - let mockResults = { - var res: [ShellOutCommand: String] = [ - .gitListTags: refs.filter(\.isTag).map { "\($0)" }.joined(separator: "\n"), - .gitCommitCount: "12", - .gitFirstCommitDate: "0", - .gitLastCommitDate: "1", - .gitShortlog : """ - 10\tPerson 1 - 2\tPerson 2 - """ - ] - for (idx, ref) in refs.enumerated() { - res[.gitRevisionInfo(reference: ref)] = "sha-\(idx)" - } - return res - }() - - let commands = QueueIsolated<[Command]>([]) - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in commands.withValue { $0.append(.init(command: cmd, path: path)!) } @@ -460,25 +447,32 @@ class AnalyzerTests: AppTestCase { throw Error() } else { return #""" - { - "name": "foo-2", - "products": [ - { - "name": "p1", - "targets": ["t1"], - "type": { - "executable": null - } - } - ], - "targets": [{"name": "t1", "type": "executable"}] - } - """# + { + "name": "foo-2", + "products": [ + { + "name": "p1", + "targets": ["t1"], + "type": { + "executable": null + } + } + ], + "targets": [{"name": "t1", "type": "executable"}] + } + """# } } return "" } + } operation: { + // setup + let urls = ["https://github.com/foo/1", "https://github.com/foo/2"] + let pkgs = try await savePackages(on: app.db, urls.asURLs, processingStage: .ingestion) + for p in pkgs { + try await Repository(package: p, defaultBranch: "main").save(on: app.db) + } // MUT try await Analyze.analyze(client: app.client, @@ -495,17 +489,17 @@ class AnalyzerTests: AppTestCase { @MainActor func test_refreshCheckout() async throws { + let commands = QueueIsolated<[String]>([]) try await withDependencies { $0.fileManager.fileExists = { @Sendable _ in true } + $0.shell.run = { @Sendable cmd, path in + commands.withValue { $0.append(cmd.description) } + return "" + } } operation: { // setup let pkg = try await savePackage(on: app.db, "1".asGithubUrl.url) try await Repository(package: pkg, defaultBranch: "main").save(on: app.db) - let commands = QueueIsolated<[String]>([]) - Current.shell.run = { @Sendable cmd, path in - commands.withValue { $0.append(cmd.description) } - return "" - } let jpr = try await Package.fetchCandidate(app.db, id: pkg.id!) // MUT @@ -528,9 +522,9 @@ class AnalyzerTests: AppTestCase { 2\tPerson 2 """ } + $0.shell.run = { @Sendable cmd, _ in throw TestError.unknownCommand } } operation: { // setup - Current.shell.run = { @Sendable cmd, _ in throw TestError.unknownCommand } let pkg = Package(id: .id0, url: "1".asGithubUrl.url) try await pkg.save(on: app.db) try await Repository(id: .id1, package: pkg, defaultBranch: "main").save(on: app.db) @@ -629,9 +623,9 @@ class AnalyzerTests: AppTestCase { if ref == .tag(1, 2, 3) { return .init(commit: "sha.1.2.3", date: .t1) } fatalError("unknown ref: \(ref)") } + $0.shell.run = { @Sendable cmd, _ in throw TestError.unknownCommand } } operation: { //setup - Current.shell.run = { @Sendable cmd, _ in throw TestError.unknownCommand } let pkgId = UUID() do { let pkg = Package(id: pkgId, url: "1".asGithubUrl.url) @@ -758,13 +752,11 @@ class AnalyzerTests: AppTestCase { func test_getPackageInfo() async throws { // Tests getPackageInfo(package:version:) + let commands = QueueIsolated<[String]>([]) try await withDependencies { $0.environment.loadSPIManifest = { _ in nil } $0.fileManager.fileExists = { @Sendable _ in true } - } operation: { - // setup - let commands = QueueIsolated<[String]>([]) - Current.shell.run = { @Sendable cmd, _ in + $0.shell.run = { @Sendable cmd, _ in commands.withValue { $0.append(cmd.description) } @@ -773,6 +765,8 @@ class AnalyzerTests: AppTestCase { } return "" } + } operation: { + // setup let pkg = try await savePackage(on: app.db, "https://github.com/foo/1") try await Repository(package: pkg, name: "1", owner: "foo").save(on: app.db) let version = try Version(id: UUID(), package: pkg, reference: .tag(.init(0, 4, 2))) @@ -913,35 +907,35 @@ class AnalyzerTests: AppTestCase { 2\tPerson 2 """ } - } operation: { - // setup - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd.description.hasSuffix("swift package dump-package") { return #""" - { - "name": "foo", - "products": [ { - "name": "p1", - "targets": [], - "type": { - "executable": null - } - }, - { - "name": "p2", - "targets": [], - "type": { - "executable": null - } + "name": "foo", + "products": [ + { + "name": "p1", + "targets": [], + "type": { + "executable": null + } + }, + { + "name": "p2", + "targets": [], + "type": { + "executable": null + } + } + ], + "targets": [] } - ], - "targets": [] - } - """# + """# } return "" } + } operation: { + // setup let pkgs = try await savePackages(on: app.db, ["1", "2"].asGithubUrls.asURLs, processingStage: .ingestion) for pkg in pkgs { try await Repository(package: pkg, defaultBranch: "main").save(on: app.db) @@ -965,21 +959,20 @@ class AnalyzerTests: AppTestCase { func test_issue_70() async throws { // Certain git commands fail when index.lock exists // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/70 + let commands = QueueIsolated<[String]>([]) try await withDependencies { // claim every file exists, including our ficticious 'index.lock' for which // we want to trigger the cleanup mechanism $0.fileManager.fileExists = { @Sendable path in true } + $0.shell.run = { @Sendable cmd, path in + commands.withValue { $0.append(cmd.description) } + return "" + } } operation: { // setup try await savePackage(on: app.db, "1".asGithubUrl.url, processingStage: .ingestion) let pkgs = try await Package.fetchCandidates(app.db, for: .analysis, limit: 10) - let commands = QueueIsolated<[String]>([]) - Current.shell.run = { @Sendable cmd, path in - commands.withValue { $0.append(cmd.description) } - return "" - } - // MUT let res = await pkgs.mapAsync { @Sendable pkg in await Result { @@ -997,23 +990,22 @@ class AnalyzerTests: AppTestCase { func test_issue_498() async throws { // git checkout can still fail despite git reset --hard + git clean // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/498 + let commands = QueueIsolated<[String]>([]) try await withDependencies { // claim every file exists, including our ficticious 'index.lock' for which // we want to trigger the cleanup mechanism $0.fileManager.fileExists = { @Sendable path in true } - } operation: { - // setup - try await savePackage(on: app.db, "1".asGithubUrl.url, processingStage: .ingestion) - let pkgs = try await Package.fetchCandidates(app.db, for: .analysis, limit: 10) - - let commands = QueueIsolated<[String]>([]) - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in commands.withValue { $0.append(cmd.description) } if cmd == .gitCheckout(branch: "master") { throw TestError.simulatedCheckoutError } return "" } + } operation: { + // setup + try await savePackage(on: app.db, "1".asGithubUrl.url, processingStage: .ingestion) + let pkgs = try await Package.fetchCandidates(app.db, for: .analysis, limit: 10) // MUT let res = await pkgs.mapAsync { @Sendable pkg in @@ -1034,9 +1026,9 @@ class AnalyzerTests: AppTestCase { // points to the correct version of Xcode! try await withDependencies { $0.fileManager.fileExists = FileManagerClient.liveValue.fileExists(atPath:) + $0.shell = .liveValue } operation: { // setup - Current.shell = .live try await withTempDir { tempDir in let fixture = fixturesDirectory() .appendingPathComponent("5.4-Package-swift").path @@ -1055,9 +1047,9 @@ class AnalyzerTests: AppTestCase { // See also https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/1441 try await withDependencies { $0.fileManager.fileExists = FileManagerClient.liveValue.fileExists(atPath:) + $0.shell = .liveValue } operation: { // setup - Current.shell = .live try await withTempDir { tempDir in let fixture = fixturesDirectory() .appendingPathComponent("5.5-Package-swift").path @@ -1075,9 +1067,9 @@ class AnalyzerTests: AppTestCase { // points to the correct version of Xcode! try await withDependencies { $0.fileManager.fileExists = FileManagerClient.liveValue.fileExists(atPath:) + $0.shell = .liveValue } operation: { // setup - Current.shell = .live try await withTempDir { tempDir in let fixture = fixturesDirectory() .appendingPathComponent("5.9-Package-swift").path @@ -1102,13 +1094,12 @@ class AnalyzerTests: AppTestCase { // NB: If this test fails on macOS make sure xcode-select -p // points to the correct version of Xcode! // setup - Current.shell = .live try await withTempDir { @Sendable tempDir in let fixture = fixturesDirectory() .appendingPathComponent("5.9-Package-swift").path let fname = tempDir.appending("/Package.swift") try await ShellOut.shellOut(to: .copyFile(from: fixture, to: fname)) - var json = try await Current.shell.run(command: .swiftDumpPackage, at: tempDir) + var json = try await ShellClient.liveValue.run(command: .swiftDumpPackage, at: tempDir) do { // "root" references tempDir's absolute path - replace it to make the test stable if var obj = try JSONSerialization.jsonObject(with: Data(json.utf8)) as? [String: Any], var packageKind = obj["packageKind"] as? [String: Any] { @@ -1158,8 +1149,14 @@ class AnalyzerTests: AppTestCase { func test_issue_693() async throws { // Handle moved tags // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/693 + let commands = QueueIsolated<[String]>([]) try await withDependencies { $0.fileManager.fileExists = { @Sendable _ in true } + $0.shell.run = { @Sendable cmd, _ in + commands.withValue { $0.append(cmd.description) } + if cmd == .gitFetchAndPruneTags { throw TestError.simulatedFetchError } + return "" + } } operation: { // setup do { @@ -1167,13 +1164,6 @@ class AnalyzerTests: AppTestCase { try await Repository(package: pkg, defaultBranch: "main").save(on: app.db) } let pkg = try await Package.fetchCandidate(app.db, id: .id0) - let commands = QueueIsolated<[String]>([]) - Current.shell.run = { @Sendable cmd, _ in - commands.withValue { $0.append(cmd.description) } - if cmd == .gitFetchAndPruneTags { throw TestError.simulatedFetchError } - return "" - } - // MUT _ = try await Analyze.refreshCheckout(package: pkg) @@ -1245,33 +1235,31 @@ class AnalyzerTests: AppTestCase { func test_issue_914() async throws { // Ensure we handle 404 repos properly // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/914 + // setup + let checkoutDir = "/checkouts" + let url = "1".asGithubUrl.url + let repoDir = try await { + let pkg = Package.init(url: url, processingStage: .ingestion) + try await pkg.save(on: app.db) + return try checkoutDir + "/" + XCTUnwrap(pkg.cacheDirectoryName) + }() + let lastUpdated = Date() + try await withDependencies { $0.fileManager.fileExists = { @Sendable path in if path.hasSuffix("github.com-foo-1") { return false } return true } - } operation: { - // setup - let checkoutDir = "/checkouts" - do { - let url = "1".asGithubUrl.url - let pkg = Package.init(url: url, processingStage: .ingestion) - try await pkg.save(on: app.db) - let repoDir = try checkoutDir + "/" + XCTUnwrap(pkg.cacheDirectoryName) - struct ShellOutError: Error {} - Current.shell.run = { @Sendable cmd, path in - if cmd == .gitClone(url: url, to: repoDir) { - throw ShellOutError() - } - throw TestError.unknownCommand + $0.shell.run = { @Sendable cmd, path in + if cmd == .gitClone(url: url, to: repoDir) { + struct ShellOutError: Error {} + throw ShellOutError() } + throw TestError.unknownCommand } - let lastUpdated = Date() - + } operation: { // MUT - try await Analyze.analyze(client: app.client, - database: app.db, - mode: .limit(10)) + try await Analyze.analyze(client: app.client, database: app.db, mode: .limit(10)) // validate let pkg = try await Package.query(on: app.db).first().unwrap() @@ -1318,6 +1306,7 @@ class AnalyzerTests: AppTestCase { 1\tPerson 2 """ } + $0.shell.run = { @Sendable _, _ in "" } } operation: { let pkgId = UUID() let pkg = Package(id: pkgId, url: "1".asGithubUrl.url, processingStage: .ingestion) @@ -1338,7 +1327,6 @@ class AnalyzerTests: AppTestCase { latest: .release, packageName: "foo-1", reference: .tag(1, 0, 0)).save(on: app.db) - Current.shell.run = { @Sendable cmd, path in "" } try await withDependencies { // first scenario: bad getTags $0.git.revisionInfo = { @Sendable _, _ in .init(commit: "", date: .t1) } @@ -1390,9 +1378,8 @@ class AnalyzerTests: AppTestCase { try await withDependencies { // third scenario: everything throws $0.git.getTags = { @Sendable _ in throw TestError.unspecifiedError } $0.git.revisionInfo = { @Sendable _, _ in throw TestError.unspecifiedError } + $0.shell.run = { @Sendable _, _ in throw TestError.unspecifiedError } } operation: { - Current.shell.run = { @Sendable _, _ in throw TestError.unspecifiedError } - // MUT try await Analyze.analyze(client: app.client, database: app.db, @@ -1424,6 +1411,7 @@ class AnalyzerTests: AppTestCase { 1\tPerson 2 """ } + $0.shell.run = { @Sendable _, _ in return "" } } operation: { let pkgId = UUID() let pkg = Package(id: pkgId, url: "1".asGithubUrl.url, processingStage: .ingestion) @@ -1445,7 +1433,6 @@ class AnalyzerTests: AppTestCase { packageName: "foo-1", reference: .tag(1, 0, 0)).save(on: app.db) struct Error: Swift.Error { } - Current.shell.run = { @Sendable cmd, path in return "" } try await withDependencies { // ensure happy path passes test (no revision changes) $0.git.revisionInfo = { @Sendable ref, _ in @@ -1488,15 +1475,14 @@ class AnalyzerTests: AppTestCase { throw Error() } } - } operation: { - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in // simulate error in getPackageInfo by failing checkout if cmd == .gitCheckout(branch: "main") { throw Error() } return "" } - + } operation: { // MUT try await Analyze.analyze(client: app.client, database: app.db, @@ -1532,6 +1518,10 @@ class AnalyzerTests: AppTestCase { $0.git.lastCommitDate = { @Sendable _ in .t1 } $0.git.revisionInfo = { @Sendable _, _ in .init(commit: "sha1", date: .t0) } $0.git.shortlog = { @Sendable _ in "10\tPerson 1" } + $0.shell.run = { @Sendable cmd, path in + if cmd == .swiftDumpPackage { return .packageDump(name: "foo1") } + return "" + } } operation: { // setup let pkg = try await savePackage(on: app.db, id: .id0, "https://github.com/foo/1".url, processingStage: .ingestion) @@ -1540,10 +1530,6 @@ class AnalyzerTests: AppTestCase { name: "1", owner: "foo", stars: 100).save(on: app.db) - Current.shell.run = { @Sendable cmd, path in - if cmd == .swiftDumpPackage { return .packageDump(name: "foo1") } - return "" - } // MUT and validation diff --git a/Tests/AppTests/ErrorReportingTests.swift b/Tests/AppTests/ErrorReportingTests.swift index 51f4ca5d2..ea4f66a57 100644 --- a/Tests/AppTests/ErrorReportingTests.swift +++ b/Tests/AppTests/ErrorReportingTests.swift @@ -53,15 +53,15 @@ class ErrorReportingTests: AppTestCase { func test_Analyzer_error_reporting() async throws { try await withDependencies { $0.fileManager.fileExists = { @Sendable _ in true } - } operation: { - // setup - try await Package(id: .id1, url: "1".asGithubUrl.url, processingStage: .ingestion).save(on: app.db) - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, _ in if cmd.description == "git tag" { return "1.0.0" } // returning a blank string will cause an exception when trying to // decode it as the manifest result - we use this to simulate errors return "invalid" } + } operation: { + // setup + try await Package(id: .id1, url: "1".asGithubUrl.url, processingStage: .ingestion).save(on: app.db) // MUT try await Analyze.analyze(client: app.client, diff --git a/Tests/AppTests/GitTests.swift b/Tests/AppTests/GitTests.swift index 1d57f1255..96f649e9e 100644 --- a/Tests/AppTests/GitTests.swift +++ b/Tests/AppTests/GitTests.swift @@ -14,57 +14,67 @@ @testable import App +import Dependencies import ShellOut import XCTVapor class GitTests: XCTestCase { - - + + func test_tag() async throws { - Current.shell.run = mock(for: "git tag", """ - test - 1.0.0-pre - 1.0.0 - 1.0.1 - 1.0.2 - """ - ) - try await XCTAssertEqualAsync( - try await Git.getTags(at: "ignored"), [ - .tag(.init(1, 0, 0, "pre")), - .tag(.init(1, 0, 0)), - .tag(.init(1, 0, 1)), - .tag(.init(1, 0, 2)), - ]) + try await withDependencies { + $0.shell.run = mock(for: "git tag", """ + test + 1.0.0-pre + 1.0.0 + 1.0.1 + 1.0.2 + """ + ) + } operation: { + try await XCTAssertEqualAsync( + try await Git.getTags(at: "ignored"), [ + .tag(.init(1, 0, 0, "pre")), + .tag(.init(1, 0, 0)), + .tag(.init(1, 0, 1)), + .tag(.init(1, 0, 2)), + ]) + } } - + func test_revInfo() async throws { - Current.shell.run = { @Sendable cmd, _ in - if cmd.description == #"git log -n1 --format=tformat:"%H-%ct" 2.2.1"# { - return "63c973f3c2e632a340936c285e94d59f9ffb01d5-1536799579" + try await withDependencies { + $0.shell.run = { @Sendable cmd, _ in + if cmd.description == #"git log -n1 --format=tformat:"%H-%ct" 2.2.1"# { + return "63c973f3c2e632a340936c285e94d59f9ffb01d5-1536799579" + } + throw TestError.unknownCommand } - throw TestError.unknownCommand + } operation: { + try await XCTAssertEqualAsync(try await Git.revisionInfo(.tag(.init(2, 2, 1)), at: "ignored"), + .init(commit: "63c973f3c2e632a340936c285e94d59f9ffb01d5", + date: Date(timeIntervalSince1970: 1536799579))) } - try await XCTAssertEqualAsync(try await Git.revisionInfo(.tag(.init(2, 2, 1)), at: "ignored"), - .init(commit: "63c973f3c2e632a340936c285e94d59f9ffb01d5", - date: Date(timeIntervalSince1970: 1536799579))) } - + func test_revInfo_tagName() async throws { // Ensure we look up by tag name and not semver // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/139 - Current.shell.run = { @Sendable cmd, _ in - if cmd.description == #"git log -n1 --format=tformat:"%H-%ct" v2.2.1"# { - return "63c973f3c2e632a340936c285e94d59f9ffb01d5-1536799579" + try await withDependencies { + $0.shell.run = { @Sendable cmd, _ in + if cmd.description == #"git log -n1 --format=tformat:"%H-%ct" v2.2.1"# { + return "63c973f3c2e632a340936c285e94d59f9ffb01d5-1536799579" + } + throw TestError.unknownCommand } - throw TestError.unknownCommand + } operation: { + try await XCTAssertEqualAsync(try await Git.revisionInfo(.tag(.init(2, 2, 1), "v2.2.1"), at: "ignored"), + .init(commit: "63c973f3c2e632a340936c285e94d59f9ffb01d5", + date: Date(timeIntervalSince1970: 1536799579))) } - try await XCTAssertEqualAsync(try await Git.revisionInfo(.tag(.init(2, 2, 1), "v2.2.1"), at: "ignored"), - .init(commit: "63c973f3c2e632a340936c285e94d59f9ffb01d5", - date: Date(timeIntervalSince1970: 1536799579))) } - + } diff --git a/Tests/AppTests/IngestionTests.swift b/Tests/AppTests/IngestionTests.swift index f7ded9024..cc15452d1 100644 --- a/Tests/AppTests/IngestionTests.swift +++ b/Tests/AppTests/IngestionTests.swift @@ -440,14 +440,13 @@ class IngestionTests: AppTestCase { $0.git.lastCommitDate = { @Sendable _ in .t0 } $0.git.revisionInfo = { @Sendable _, _ in .init(commit: "sha0", date: .t0) } $0.git.shortlog = { @Sendable _ in "" } - } operation: { [db = app.db] in - Current.shell.run = { @Sendable cmd, _ in + $0.shell.run = { @Sendable cmd, _ in if cmd.description.hasSuffix("package dump-package") { return .packageDump(name: "foo") } return "" } - + } operation: { [db = app.db] in try await Analyze.analyze(client: app.client, database: db, mode: .id(.id0)) try await Analyze.analyze(client: app.client, database: db, mode: .id(.id1)) try await XCTAssertEqualAsync(try await Package.find(.id0, on: db)?.processingStage, .analysis) diff --git a/Tests/AppTests/MastodonTests.swift b/Tests/AppTests/MastodonTests.swift index 165f75041..33184eabc 100644 --- a/Tests/AppTests/MastodonTests.swift +++ b/Tests/AppTests/MastodonTests.swift @@ -50,15 +50,15 @@ final class MastodonTests: AppTestCase { XCTFail("message must only be set once") } } - } operation: { - // setup - let url = "https://github.com/foo/bar" - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd.description.hasSuffix("swift package dump-package") { return #"{ "name": "Mock", "products": [], "targets": [] }"# } return "" } + } operation: { + // setup + let url = "https://github.com/foo/bar" try await withDependencies { $0.date.now = .now diff --git a/Tests/AppTests/Mocks/AppShell+mock.swift b/Tests/AppTests/Mocks/AppShell+mock.swift index d802806f8..f3354dfaa 100644 --- a/Tests/AppTests/Mocks/AppShell+mock.swift +++ b/Tests/AppTests/Mocks/AppShell+mock.swift @@ -16,5 +16,6 @@ extension App.Shell { - static let mock: Self = .init(run: { cmd, path in "" }) + @available(*, deprecated) + static let mock: Self = .init() } diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift index a2c28e983..5d69fd750 100644 --- a/Tests/AppTests/PackageController+routesTests.swift +++ b/Tests/AppTests/PackageController+routesTests.swift @@ -1516,6 +1516,10 @@ class PackageController_routesTests: SnapshotTestCase { } $0.git.shortlog = { @Sendable _ in "2\tauthor" } $0.httpClient.fetchDocumentation = { @Sendable _ in .ok(body: .mockIndexHTML()) } + $0.shell.run = { @Sendable cmd, _ in + if cmd.description == "swift package dump-package" { return .mockManifest } + return "" + } $0.timeZone = .utc } operation: { // setup @@ -1530,10 +1534,6 @@ class PackageController_routesTests: SnapshotTestCase { packageName: "bar", reference: .branch("main")) .save(on: app.db) - Current.shell.run = { @Sendable cmd, _ in - if cmd.description == "swift package dump-package" { return .mockManifest } - return "" - } // Make sure the new commit doesn't get throttled try await withDependencies { $0.date.now = .t1 + Constants.branchVersionRefreshDelay + 1 diff --git a/Tests/AppTests/PackageTests.swift b/Tests/AppTests/PackageTests.swift index 6bf2632bb..dd18517b9 100644 --- a/Tests/AppTests/PackageTests.swift +++ b/Tests/AppTests/PackageTests.swift @@ -330,14 +330,14 @@ final class PackageTests: AppTestCase { $0.packageListRepository.fetchPackageList = { @Sendable _ in [url.url] } $0.packageListRepository.fetchPackageDenyList = { @Sendable _ in [] } $0.packageListRepository.fetchCustomCollections = { @Sendable _ in [] } - } operation: { - // setup - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd.description.hasSuffix("swift package dump-package") { return #"{ "name": "Mock", "products": [] }"# } return "" } + } operation: { + // setup let db = app.db // run reconcile to ingest package try await reconcile(client: app.client, database: app.db) diff --git a/Tests/AppTests/PipelineTests.swift b/Tests/AppTests/PipelineTests.swift index 002c2e0da..23481bc9f 100644 --- a/Tests/AppTests/PipelineTests.swift +++ b/Tests/AppTests/PipelineTests.swift @@ -183,15 +183,13 @@ class PipelineTests: AppTestCase { $0.packageListRepository.fetchPackageDenyList = { @Sendable _ in [] } $0.packageListRepository.fetchCustomCollections = { @Sendable _ in [] } $0.packageListRepository.fetchCustomCollection = { @Sendable _, _ in [] } - } operation: { - // setup - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd.description.hasSuffix("swift package dump-package") { return #"{ "name": "Mock", "products": [], "targets": [] }"# } return "" } - + } operation: { // MUT - first stage try await reconcile(client: app.client, database: app.db) diff --git a/Tests/AppTests/ReAnalyzeVersionsTests.swift b/Tests/AppTests/ReAnalyzeVersionsTests.swift index 53601cdbd..da9eb3aa9 100644 --- a/Tests/AppTests/ReAnalyzeVersionsTests.swift +++ b/Tests/AppTests/ReAnalyzeVersionsTests.swift @@ -63,19 +63,19 @@ class ReAnalyzeVersionsTests: AppTestCase { try await withDependencies { $0.git.revisionInfo = { @Sendable _, _ in .init(commit: "sha", date: .t0) } - } operation: { - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd.description.hasSuffix("swift package dump-package") { return #""" - { - "name": "SPI-Server", - "products": [], - "targets": [] - } - """# + { + "name": "SPI-Server", + "products": [], + "targets": [] + } + """# } return "" } + } operation: { do { // run initial analysis and assert initial state for versions try await Analyze.analyze(client: app.client, @@ -88,9 +88,9 @@ class ReAnalyzeVersionsTests: AppTestCase { XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [[], []]) XCTAssertEqual(versions.map(\.releaseNotes) , [nil, nil]) } - do { + try await withDependencies { // Update state that would normally not be affecting existing versions, effectively simulating the situation where we only started parsing it after versions had already been created - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd.description.hasSuffix("swift package dump-package") { return #""" { @@ -105,6 +105,7 @@ class ReAnalyzeVersionsTests: AppTestCase { } return "" } + } operation: { // also, update release notes to ensure mergeReleaseInfo is being called let r = try await Repository.find(repoId, on: app.db).unwrap() r.releases = [ @@ -198,12 +199,7 @@ class ReAnalyzeVersionsTests: AppTestCase { 2\tPerson 2 """ } - } operation: { - let pkg = try await savePackage(on: app.db, - "https://github.com/foo/1".url, - processingStage: .ingestion) - try await Repository(package: pkg, defaultBranch: "main").save(on: app.db) - Current.shell.run = { @Sendable cmd, path in + $0.shell.run = { @Sendable cmd, path in if cmd == .swiftDumpPackage { return #""" { @@ -215,6 +211,11 @@ class ReAnalyzeVersionsTests: AppTestCase { } return "" } + } operation: { + let pkg = try await savePackage(on: app.db, + "https://github.com/foo/1".url, + processingStage: .ingestion) + try await Repository(package: pkg, defaultBranch: "main").save(on: app.db) try await Analyze.analyze(client: app.client, database: app.db, mode: .limit(10)) @@ -225,26 +226,28 @@ class ReAnalyzeVersionsTests: AppTestCase { XCTAssertEqual(candidates.count, 1) } - Current.shell.run = { @Sendable cmd, path in - if cmd == .swiftDumpPackage { - // simulate error during package dump - struct Error: Swift.Error { } - throw Error() + try await withDependencies { + $0.shell.run = { @Sendable cmd, path in + if cmd == .swiftDumpPackage { + // simulate error during package dump + struct Error: Swift.Error { } + throw Error() + } + return "" } - return "" - } - - // MUT - try await ReAnalyzeVersions.reAnalyzeVersions(client: app.client, - database: app.db, - before: Date.now, - refreshCheckouts: false, - limit: 10) + } operation: { + // MUT + try await ReAnalyzeVersions.reAnalyzeVersions(client: app.client, + database: app.db, + before: Date.now, + refreshCheckouts: false, + limit: 10) - // validate - let candidates = try await Package - .fetchReAnalysisCandidates(app.db, before: cutoff, limit: 10) - XCTAssertEqual(candidates.count, 0) + // validate + let candidates = try await Package + .fetchReAnalysisCandidates(app.db, before: cutoff, limit: 10) + XCTAssertEqual(candidates.count, 0) + } } } From 10fb3062b5ca19a90d76c69d5f01c15d726489bd Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 29 Jan 2025 09:58:36 +0100 Subject: [PATCH 3/4] Update tests --- Tests/AppTests/AnalyzerTests.swift | 2 +- Tests/AppTests/GitLiveTests.swift | 9 ++++ Tests/AppTests/ReAnalyzeVersionsTests.swift | 53 +++++++++++---------- 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/Tests/AppTests/AnalyzerTests.swift b/Tests/AppTests/AnalyzerTests.swift index c09a5e2d6..ff1a7f7f7 100644 --- a/Tests/AppTests/AnalyzerTests.swift +++ b/Tests/AppTests/AnalyzerTests.swift @@ -317,6 +317,7 @@ class AnalyzerTests: AppTestCase { $0.git.hasBranch = { @Sendable _, _ in false } // simulate analysis error via branch mismatch $0.git.lastCommitDate = { @Sendable _ in .t1 } $0.git.shortlog = { @Sendable _ in "" } + $0.shell.run = { @Sendable _, _ in "" } } operation: { // setup do { @@ -324,7 +325,6 @@ class AnalyzerTests: AppTestCase { try await Repository(package: pkg, defaultBranch: "main").save(on: app.db) } - // Ensure candidate selection is as expected let app = self.app! try await XCTAssertEqualAsync(try await Package.fetchCandidates(app.db, for: .ingestion, limit: 10).count, 0) diff --git a/Tests/AppTests/GitLiveTests.swift b/Tests/AppTests/GitLiveTests.swift index 674cea11a..80515b4c3 100644 --- a/Tests/AppTests/GitLiveTests.swift +++ b/Tests/AppTests/GitLiveTests.swift @@ -16,6 +16,7 @@ import XCTest @testable import App +import Dependencies import ShellOut @@ -47,6 +48,14 @@ class GitLiveTests: XCTestCase { override func setUp() { Current.setLogger(.init(label: "test", factory: { _ in logger })) } + + override func invokeTest() { + withDependencies { + $0.shell = .liveValue + } operation: { + super.invokeTest() + } + } } diff --git a/Tests/AppTests/ReAnalyzeVersionsTests.swift b/Tests/AppTests/ReAnalyzeVersionsTests.swift index da9eb3aa9..0b197568e 100644 --- a/Tests/AppTests/ReAnalyzeVersionsTests.swift +++ b/Tests/AppTests/ReAnalyzeVersionsTests.swift @@ -88,6 +88,7 @@ class ReAnalyzeVersionsTests: AppTestCase { XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [[], []]) XCTAssertEqual(versions.map(\.releaseNotes) , [nil, nil]) } + try await withDependencies { // Update state that would normally not be affecting existing versions, effectively simulating the situation where we only started parsing it after versions had already been created $0.shell.run = { @Sendable cmd, path in @@ -105,42 +106,42 @@ class ReAnalyzeVersionsTests: AppTestCase { } return "" } - } operation: { // also, update release notes to ensure mergeReleaseInfo is being called let r = try await Repository.find(repoId, on: app.db).unwrap() r.releases = [ .mock(description: "rel 1.2.3", tagName: "1.2.3") ] try await r.save(on: app.db) - } - do { // assert running analysis again does not update existing versions - try await Analyze.analyze(client: app.client, - database: app.db, - mode: .limit(10)) + } operation: { + do { // assert running analysis again does not update existing versions + try await Analyze.analyze(client: app.client, + database: app.db, + mode: .limit(10)) + let versions = try await Version.query(on: app.db) + .with(\.$targets) + .all() + XCTAssertEqual(versions.map(\.toolsVersion), [nil, nil]) + XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [[], []]) + XCTAssertEqual(versions.map(\.releaseNotes) , [nil, nil]) + XCTAssertEqual(versions.map(\.docArchives), [nil, nil]) + } + + // MUT + try await ReAnalyzeVersions.reAnalyzeVersions(client: app.client, + database: app.db, + before: Date.now, + refreshCheckouts: false, + limit: 10) + + // validate that re-analysis has now updated existing versions let versions = try await Version.query(on: app.db) .with(\.$targets) + .sort(\.$createdAt) .all() - XCTAssertEqual(versions.map(\.toolsVersion), [nil, nil]) - XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [[], []]) - XCTAssertEqual(versions.map(\.releaseNotes) , [nil, nil]) - XCTAssertEqual(versions.map(\.docArchives), [nil, nil]) + XCTAssertEqual(versions.map(\.toolsVersion), ["5.3", "5.3"]) + XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [["t1"], ["t1"]]) + XCTAssertEqual(versions.compactMap(\.releaseNotes) , ["rel 1.2.3"]) } - - // MUT - try await ReAnalyzeVersions.reAnalyzeVersions(client: app.client, - database: app.db, - before: Date.now, - refreshCheckouts: false, - limit: 10) - - // validate that re-analysis has now updated existing versions - let versions = try await Version.query(on: app.db) - .with(\.$targets) - .sort(\.$createdAt) - .all() - XCTAssertEqual(versions.map(\.toolsVersion), ["5.3", "5.3"]) - XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [["t1"], ["t1"]]) - XCTAssertEqual(versions.compactMap(\.releaseNotes) , ["rel 1.2.3"]) } } } From b574060b552f52fdeba04d217282a12c761d628b Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 29 Jan 2025 10:15:31 +0100 Subject: [PATCH 4/4] Remove Current.shell --- Sources/App/Core/AppEnvironment.swift | 12 +---------- Tests/AppTests/GitLiveTests.swift | 1 - .../AppTests/Mocks/AppEnvironment+mock.swift | 3 +-- Tests/AppTests/Mocks/AppShell+mock.swift | 21 ------------------- Tests/AppTests/ReAnalyzeVersionsTests.swift | 2 +- 5 files changed, 3 insertions(+), 36 deletions(-) delete mode 100644 Tests/AppTests/Mocks/AppShell+mock.swift diff --git a/Sources/App/Core/AppEnvironment.swift b/Sources/App/Core/AppEnvironment.swift index 5d8975266..0ab71acf8 100644 --- a/Sources/App/Core/AppEnvironment.swift +++ b/Sources/App/Core/AppEnvironment.swift @@ -25,7 +25,6 @@ import FoundationNetworking struct AppEnvironment: Sendable { var logger: @Sendable () -> Logger var setLogger: @Sendable (Logger) -> Void - var shell: Shell } @@ -34,16 +33,7 @@ extension AppEnvironment { static let live = AppEnvironment( logger: { logger }, - setLogger: { logger in Self.logger = logger }, - shell: .live - ) -} - - - -@available(*, deprecated) -struct Shell: Sendable { - static let live: Self = .init( + setLogger: { logger in Self.logger = logger } ) } diff --git a/Tests/AppTests/GitLiveTests.swift b/Tests/AppTests/GitLiveTests.swift index 80515b4c3..4ded55d75 100644 --- a/Tests/AppTests/GitLiveTests.swift +++ b/Tests/AppTests/GitLiveTests.swift @@ -34,7 +34,6 @@ class GitLiveTests: XCTestCase { // Simulate a class setUp (which does not exist as an async function) if Self.hasRunSetup { return } Self.hasRunSetup = true - Current.shell = .live try! Foundation.FileManager.default.createDirectory(atPath: Self.tempDir, withIntermediateDirectories: false, attributes: nil) try! await ShellOut.shellOut(to: .init(command: "unzip", arguments: [Self.sampleGitRepoZipFile]), at: Self.tempDir) } diff --git a/Tests/AppTests/Mocks/AppEnvironment+mock.swift b/Tests/AppTests/Mocks/AppEnvironment+mock.swift index 7ece9ebd3..a15760ec1 100644 --- a/Tests/AppTests/Mocks/AppEnvironment+mock.swift +++ b/Tests/AppTests/Mocks/AppEnvironment+mock.swift @@ -23,8 +23,7 @@ extension AppEnvironment { static func mock(eventLoop: EventLoop) -> Self { .init( logger: { logger }, - setLogger: { logger in Self.logger = logger }, - shell: .mock + setLogger: { logger in Self.logger = logger } ) } } diff --git a/Tests/AppTests/Mocks/AppShell+mock.swift b/Tests/AppTests/Mocks/AppShell+mock.swift deleted file mode 100644 index f3354dfaa..000000000 --- a/Tests/AppTests/Mocks/AppShell+mock.swift +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright Dave Verwer, Sven A. Schmidt, and other contributors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -@testable import App - - -extension App.Shell { - @available(*, deprecated) - static let mock: Self = .init() -} diff --git a/Tests/AppTests/ReAnalyzeVersionsTests.swift b/Tests/AppTests/ReAnalyzeVersionsTests.swift index 0b197568e..ff8de0c54 100644 --- a/Tests/AppTests/ReAnalyzeVersionsTests.swift +++ b/Tests/AppTests/ReAnalyzeVersionsTests.swift @@ -88,7 +88,7 @@ class ReAnalyzeVersionsTests: AppTestCase { XCTAssertEqual(versions.map { $0.targets.map(\.name) } , [[], []]) XCTAssertEqual(versions.map(\.releaseNotes) , [nil, nil]) } - + try await withDependencies { // Update state that would normally not be affecting existing versions, effectively simulating the situation where we only started parsing it after versions had already been created $0.shell.run = { @Sendable cmd, path in