Update test_ingest_unique_owner_name_violation to reflect new error handling

Author: finestructure

Sources/App/Commands/Common.swift

@@ -185,9 +185,8 @@ func recordIngestionError(database: Database, error: Ingestion.Error) async thro
             try await Package
                 .update(for: error.packageId, on: database, status: .invalidUrl, stage: .ingestion)
         case .repositorySaveUniqueViolation:
-            // Speficically do _not_ update package at all - this is what test_ingest_unique_owner_name_violation expects
-#warning("check what are the consequences if we do? Does this break ingestion somehow?")
-            break
+            try await Package
+                .update(for: error.packageId, on: database, status: .ingestionFailed, stage: .ingestion)
     }
 }
 

Tests/AppTests/IngestorTests.swift

@@ -363,9 +363,10 @@ class IngestorTests: AppTestCase {
         //   - don't update package
         //   - don't create repository records
         // setup
-        for url in ["https://github.com/foo/1", "https://github.com/foo/2"].asURLs {
-            try await Package(url: url, processingStage: .reconciliation).save(on: app.db)
-        }
+        try await Package(id: .id0, url: "https://github.com/foo/0", status: .ok, processingStage: .reconciliation)
+            .save(on: app.db)
+        try await Package(id: .id1, url: "https://github.com/foo/1", status: .ok, processingStage: .reconciliation)
+            .save(on: app.db)
         // Return identical metadata for both packages, same as a for instance a redirected
         // package would after a rename / ownership change
         Current.fetchMetadata = { _, _, _ in
@@ -385,7 +386,6 @@ class IngestorTests: AppTestCase {
                 stars: 0,
                 summary: "desc")
         }
-        let lastUpdate = Date()
 
         try await withDependencies {
             $0.date.now = .now
@@ -396,31 +396,48 @@ class IngestorTests: AppTestCase {
 
         // validate repositories (single element pointing to the ingested package)
         let repos = try await Repository.query(on: app.db).all()
-        let ingested = try await Package.query(on: app.db)
-            .filter(\.$processingStage == .ingestion)
+        XCTAssertEqual(repos.count, 1)
+
+        // validate packages - one should have succeeded, one should have failed
+        let succeeded = try await Package.query(on: app.db)
+            .filter(\.$status == .ok)
             .first()
             .unwrap()
-        XCTAssertEqual(repos.map(\.$package.id), [try ingested.requireID()])
-
-        // validate packages
-        let reconciled = try await Package.query(on: app.db)
-            .filter(\.$processingStage == .reconciliation)
+        let failed = try await Package.query(on: app.db)
+            .filter(\.$status == .ingestionFailed)
             .first()
             .unwrap()
-        // the ingested package has the update ...
-        XCTAssertEqual(ingested.status, .new)
-        XCTAssertEqual(ingested.processingStage, .ingestion)
-        XCTAssert(ingested.updatedAt! > lastUpdate)
-        // ... while the reconciled package remains unchanged ...
-        XCTAssertEqual(reconciled.status, .new)
-        XCTAssertEqual(reconciled.processingStage, .reconciliation)
-        XCTAssert(reconciled.updatedAt! < lastUpdate)
-        // ... and an error has been logged
+        XCTAssertEqual(succeeded.processingStage, .ingestion)
+        XCTAssertEqual(failed.processingStage, .ingestion)
+        // an error must have been logged
         try logger.logs.withValue { logs in
             XCTAssertEqual(logs.count, 1)
             let log = try XCTUnwrap(logs.first)
             XCTAssertEqual(log.level, .critical)
-            XCTAssertEqual(log.message, #"Ingestion.Error(\#(try reconciled.requireID()), repositorySaveUniqueViolation(duplicate key value violates unique constraint "idx_repositories_owner_name"))"#)
+            XCTAssertEqual(log.message, #"Ingestion.Error(\#(try failed.requireID()), repositorySaveUniqueViolation(duplicate key value violates unique constraint "idx_repositories_owner_name"))"#)
+        }
+
+        // ensure analysis can process these packages
+        try await withDependencies {
+            $0.date.now = .now
+            $0.environment.allowSocialPosts = { false }
+        } operation: {
+            Current.fileManager.fileExists = { @Sendable _ in true }
+            Current.git.commitCount = { @Sendable _ in 1 }
+            Current.git.firstCommitDate = { @Sendable _ in .t0 }
+            Current.git.lastCommitDate = { @Sendable _ in .t0 }
+            Current.git.getTags = { @Sendable _ in [] }
+            Current.git.hasBranch = { @Sendable _, _ in true }
+            Current.git.revisionInfo = { @Sendable _, _ in .init(commit: "sha0", date: .t0) }
+            Current.git.shortlog = { @Sendable _ in "" }
+            Current.shell.run = { @Sendable cmd, _ in
+                if cmd.description.hasSuffix("package dump-package") {
+                    return .packageDump(name: "foo")
+                }
+                return ""
+            }
+
+            try await Analyze.analyze(client: app.client, database: app.db, mode: .limit(10))
         }
     }
 
@@ -665,3 +682,24 @@ class IngestorTests: AppTestCase {
         XCTAssertEqual(fork5, nil)
     }
 }
+
+
+private extension String {
+    static func packageDump(name: String) -> Self {
+        #"""
+            {
+              "name": "\#(name)",
+              "products": [
+                {
+                  "name": "p1",
+                  "targets": [],
+                  "type": {
+                    "executable": null
+                  }
+                }
+              ],
+              "targets": []
+            }
+            """#
+    }
+}