Remove invalid model definition from test (#203)

gwynne · web-flow · commit 44247df7b823 · 2023-03-03T19:09:22.000-06:00
* Remove invalid model definition from test that doesn't use it to begin with anyway. (Model used @id(key:) with a non-UUID value) * CI update (add concurrency spec, use API breakage workflow, add 5.8 nightly test, enable TSan) * More CI cleanup (reduce redundant jobs), reduce excess thread created in tests * Add some missing imports * Fix hanging tests behavior caused by misuse of Fluent.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -1,4 +1,7 @@
 name: test
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
 on:
   pull_request: { branches: ['*'] }
   push: { branches: ['main'] }
@@ -23,7 +26,10 @@ jobs:
   # Baseline test run for code coverage stats
   codecov:
     strategy:
-      matrix: { dbimage: ['postgres:15'], dbauth: ['scram-sha-256'] }
+      matrix:
+        include:
+          - dbimage: postgres:15
+            dbauth: scram-sha-256
     runs-on: ubuntu-latest
     container: swift:5.7-jammy
     services:
@@ -69,29 +75,21 @@ jobs:
     steps:
       - name: Check out package
         uses: actions/checkout@v3
-        with:
-          fetch-depth: 0
-      # https://github.com/actions/checkout/issues/766
-      - name: Mark the workspace as safe
-        run: git config --global --add safe.directory ${GITHUB_WORKSPACE}
-      - name: Check for API breaking changes
-        run: swift package diagnose-api-breaking-changes origin/main
+        with: { 'fetch-depth': 0 }
+      - name: Run API breakage check action
+        uses: vapor/ci/.github/actions/ci-swift-check-api-breakage@reusable-workflows
 
   linux-all:
     if: github.event_name == 'pull_request'
     strategy:
       fail-fast: false
       matrix:
-        dbimage: ['postgres:15', 'postgres:13', 'postgres:11']
-        swiftver: [
-            'swift:5.5-bionic', 'swift:5.6-focal', 'swift:5.7-jammy',
-            'swiftlang/swift:nightly-main-jammy'
-        ]
-        include: [
-            {dbimage: 'postgres:15', dbauth: 'scram-sha-256'},
-            {dbimage: 'postgres:13', dbauth: 'md5'},
-            {dbimage: 'postgres:11', dbauth: 'trust'}
-        ]
+        include:
+          - {dbimage: 'postgres:11', dbauth: 'trust',         swiftver: 'swift:5.5-bionic'}
+          - {dbimage: 'postgres:13', dbauth: 'md5',           swiftver: 'swift:5.6-focal'}
+          - {dbimage: 'postgres:15', dbauth: 'scram-sha-256', swiftver: 'swift:5.7-jammy'}
+          - {dbimage: 'postgres:15', dbauth: 'scram-sha-256', swiftver: 'swiftlang/swift:nightly-5.8-jammy'}
+          - {dbimage: 'postgres:15', dbauth: 'scram-sha-256', swiftver: 'swiftlang/swift:nightly-main-jammy'}
     container: ${{ matrix.swiftver }}
     runs-on: ubuntu-latest
     services:
@@ -115,17 +113,18 @@ jobs:
       - name: Check out package
         uses: actions/checkout@v3
       - name: Run all tests
-        run: swift test
+        run: swift test --sanitize=thread
 
   macos-all:
     if: github.event_name == 'pull_request'
     strategy:
       fail-fast: false
       matrix:
-        dbimage: ['postgresql@14'] # N.B.: As of this writing, Homebrew didn't have Postgres 15 yet
-        dbauth: ['scram-sha-256']
-        macos: ['macos-11', 'macos-12']
-        xcode: ['latest-stable']
+        include:
+          - dbimage: postgresql@14
+            dbauth: scram-sha-256
+            macos: macos-12
+            xcode: latest-stable
     runs-on: ${{ matrix.macos }}
     env:
       LOG_LEVEL: debug
@@ -159,15 +158,14 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v3
       - name: Run all tests
-        run: swift test
+        run: swift test --sanitize=thread
 
   test-exports:
     name: Test exports
     runs-on: ubuntu-latest
     steps:
       - name: Check out Vapor
         uses: actions/checkout@v3
-        with:
-          fetch-depth: 0
+        with: { 'fetch-depth': 0 }
       - name: Build
         run: swift build -Xswiftc -DBUILDING_DOCC
diff --git a/Sources/FluentPostgresDriver/Exports.swift b/Sources/FluentPostgresDriver/Exports.swift
@@ -14,4 +14,4 @@ extension DatabaseID {
     public static var psql: DatabaseID {
         return .init(string: "psql")
     }
-}
+}
diff --git a/Sources/FluentPostgresDriver/FluentPostgresDatabase.swift b/Sources/FluentPostgresDriver/FluentPostgresDatabase.swift
@@ -1,6 +1,8 @@
+import FluentKit
 import FluentSQL
 import Logging
 import PostgresKit
+import SQLKit
 
 struct _FluentPostgresDatabase {
     let database: PostgresDatabase
diff --git a/Sources/FluentPostgresDriver/PostgresConverterDelegate.swift b/Sources/FluentPostgresDriver/PostgresConverterDelegate.swift
@@ -1,4 +1,6 @@
+import FluentKit
 import FluentSQL
+import SQLKit
 
 struct PostgresConverterDelegate: SQLConverterDelegate {
     func customDataType(_ dataType: DatabaseSchema.DataType) -> SQLExpression? {
diff --git a/Sources/FluentPostgresDriver/PostgresError+Database.swift b/Sources/FluentPostgresDriver/PostgresError+Database.swift
@@ -1,3 +1,4 @@
+import FluentKit
 import FluentSQL
 import PostgresKit
 
diff --git a/Tests/FluentPostgresDriverTests/FluentPostgresDriverTests.swift b/Tests/FluentPostgresDriverTests/FluentPostgresDriverTests.swift
@@ -60,18 +60,6 @@ final class FluentPostgresDriverTests: XCTestCase {
     }
     
     func testBlob() throws {
-        final class Foo: Model {
-            static let schema = "foos"
-
-            @ID(key: "id")
-            var id: Int?
-
-            @Field(key: "data")
-            var data: [UInt8]
-
-            init() { }
-        }
-
         struct CreateFoo: Migration {
             func prepare(on database: Database) -> EventLoopFuture<Void> {
                 return database.schema("foos")
@@ -193,8 +181,8 @@ final class FluentPostgresDriverTests: XCTestCase {
         try super.setUpWithError()
         
         XCTAssert(isLoggingConfigured)
-        self.eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
-        self.threadPool = NIOThreadPool(numberOfThreads: System.coreCount)
+        self.eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: Swift.min(System.coreCount, 2))
+        self.threadPool = NIOThreadPool(numberOfThreads: 1)
         self.dbs = Databases(threadPool: threadPool, on: self.eventLoopGroup)
 
         self.dbs.use(.testPostgres(subconfig: "A"), as: .a)
diff --git a/Tests/FluentPostgresDriverTests/FluentPostgresTransactionControlTests.swift b/Tests/FluentPostgresDriverTests/FluentPostgresTransactionControlTests.swift
@@ -7,77 +7,52 @@ import PostgresKit
 
 final class FluentPostgresTransactionControlTests: XCTestCase {
     
-    func testTransactionControl() throws {
-        try (self.db as! TransactionControlDatabase).beginTransaction().wait()
-        
-        let todo1 = Todo(title: "Test")
-        let todo2 = Todo(title: "Test2")
-        try todo1.save(on: self.db).wait()
-        try todo2.save(on: self.db).wait()
-        
-        try (self.db as! TransactionControlDatabase).commitTransaction().wait()
-        
-        let count = try Todo.query(on: self.db).count().wait()
-        XCTAssertEqual(count, 2)
-    }
-    
     func testRollback() throws {
-        try (self.db as! TransactionControlDatabase).beginTransaction().wait()
-        
-        let todo1 = Todo(title: "Test")
-        
-        try todo1.save(on: self.db).wait()
-        
-        let duplicate = Todo(title: "Test")
-        var errorCaught = false
-        
         do {
-            try duplicate.create(on: self.db).wait()
+            try self.db.withConnection { db -> EventLoopFuture<Void> in
+                (db as! TransactionControlDatabase).beginTransaction().flatMap { () -> EventLoopFuture<Void> in
+                    let todo1 = Todo(title: "Test")
+                    return todo1.save(on: db)
+                }.flatMap { () -> EventLoopFuture<Void> in
+                    let duplicate = Todo(title: "Test")
+                    return duplicate.create(on: db)
+                        .flatMap {
+                            (db as! TransactionControlDatabase).commitTransaction()
+                        }.flatMapError { (e: Error) -> EventLoopFuture<Void> in
+                            return (db as! TransactionControlDatabase).rollbackTransaction()
+                                .flatMap { db.eventLoop.makeFailedFuture(e) }
+                        }
+                }
+            }.wait()
+            XCTFail("Expected error but none was thrown")
+        } catch PostgresError.server(let e) where e.fields[.sqlState] == "23505" {
+            // ignore
         } catch {
-            errorCaught = true
-            try (self.db as! TransactionControlDatabase).rollbackTransaction().wait()
+            XCTFail("Expected SQL state 23505 but got \(error)")
         }
-        
-        if !errorCaught {
-            try (self.db as! TransactionControlDatabase).commitTransaction().wait()
-        }
-        
-        XCTAssertTrue(errorCaught)
+
         let count2 = try Todo.query(on: self.db).count().wait()
         XCTAssertEqual(count2, 0)
     }
     
-    var benchmarker: FluentBenchmarker {
-        return .init(databases: self.dbs)
-    }
     var eventLoopGroup: EventLoopGroup!
     var threadPool: NIOThreadPool!
     var dbs: Databases!
-    var db: Database {
-        self.benchmarker.database
-    }
-    var postgres: PostgresDatabase {
-        self.db as! PostgresDatabase
-    }
+    var db: Database!
     
     override func setUpWithError() throws {
         try super.setUpWithError()
         
         XCTAssert(isLoggingConfigured)
-        self.eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+        self.eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: Swift.min(System.coreCount, 2))
         self.threadPool = NIOThreadPool(numberOfThreads: 1)
         self.dbs = Databases(threadPool: threadPool, on: self.eventLoopGroup)
 
         self.dbs.use(.testPostgres(subconfig: "A"), as: .a)
-        self.dbs.use(.testPostgres(subconfig: "B"), as: .b)
-
-        let a = self.dbs.database(.a, logger: Logger(label: "test.fluent.a"), on: self.eventLoopGroup.next())
-        _ = try (a as! PostgresDatabase).query("drop schema public cascade").wait()
-        _ = try (a as! PostgresDatabase).query("create schema public").wait()
 
-        let b = self.dbs.database(.b, logger: Logger(label: "test.fluent.b"), on: self.eventLoopGroup.next())
-        _ = try (b as! PostgresDatabase).query("drop schema public cascade").wait()
-        _ = try (b as! PostgresDatabase).query("create schema public").wait()
+        self.db = self.dbs.database(.a, logger: Logger(label: "test.fluent.a"), on: self.eventLoopGroup.any())
+        _ = try (self.db as! PostgresDatabase).query("drop schema public cascade").wait()
+        _ = try (self.db as! PostgresDatabase).query("create schema public").wait()
         
         try CreateTodo().prepare(on: self.db).wait()
     }
@@ -99,7 +74,7 @@ final class FluentPostgresTransactionControlTests: XCTestCase {
         @Field(key: "title")
         var title: String
 
-        init() { }
+        init() {}
         init(title: String) { self.title = title; id = nil }
     }
     

Original file line number	Diff line number	Diff line change
`@@ -14,4 +14,4 @@ extension DatabaseID {`
`14`	`14`	`public static var psql: DatabaseID {`
`15`	`15`	`return .init(string: "psql")`
`16`	`16`	`}`
`17`		`-}`
	`17`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+import FluentKit`
`1`	`2`	`import FluentSQL`
`2`	`3`	`import PostgresKit`
`3`	`4`