SWIFT-1609 Async-ifying the Unified Test Runner and underlying operations #769

rchhaya · 2022-07-29T15:12:57Z

No description provided.

…an be created, iterated, and closed

rchhaya · 2022-08-09T15:21:31Z

Apologies for a ton of minor commits for compiler directive things - tried switching swiftenv but since I cant run on terminal (weird env problems, can only build via xcode), the version switch didnt actully cascade down. Should be all fixed now + working on resolving an issue with sharding

.gitignore

Tests/MongoSwiftTests/CrudUnifiedTests.swift

Tests/MongoSwiftTests/DNSSeedlistTests.swift

Tests/MongoSwiftTests/UnifiedTestRunner/TestWriteModel.swift

Tests/MongoSwiftTests/MongoCursorTests.swift

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

kmahar · 2022-08-12T18:16:00Z

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

+#if compiler(>=5.5.2) && canImport(_Concurrency)
+import MongoSwift
 import Nimble
+import NIOPosix


do we need the NIOPosix import?

No, leftover from elg stuff during debugging. Removed.

doesn't look like this actually got removed

This is actually needed in order to use MultiThreadedEventLoopGroup in this file.

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

Tests/MongoSwiftTests/UnifiedTestRunner/Context.swift

kmahar

thanks for addressing my comments! I have a few more mostly minor ones.

Tests/MongoSwiftTests/UnifiedTestRunner/TestWriteModel.swift

Tests/MongoSwiftTests/AsyncTestUtils.swift

Tests/MongoSwiftTests/DNSSeedlistTests.swift

Tests/MongoSwiftTests/Failpoint.swift

Tests/MongoSwiftTests/UnifiedTestRunner/Context.swift

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

kmahar · 2022-08-16T03:08:30Z

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

+                    for (_, client) in clientMap {
                        for entity in collEntities {
-                            _ = try client.db(entity.namespace.db).runCommand(
+                            _ = try? await client.db(entity.namespace.db).runCommand(


hmmm, I'd still like to understand why this only shows up now and never came up when the runner was sync. it might point to some subtle behavioral difference we're missing that results from this refactor. I'd think that we should be doing the same operation in the same order we were before. could you maybe link me to an example evergreen run where this happens so I can see the output?

kmahar

couple suggestions; otherwise LGTM mod ongoing thread about needing to try? the distinct workaround

Tests/MongoSwiftTests/Failpoint.swift

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

Co-authored-by: Kaitlin Mahar <kaitlinmahar@gmail.com>

kmahar · 2022-08-18T17:42:42Z

Tests/MongoSwiftTests/Failpoint.swift

@@ -0,0 +1,49 @@
+#if compiler(>=5.5.2) && canImport(_Concurrency)
+import MongoSwift
+import NIOPosix


I think we can get rid of the NIOPosix import

Oops, remnant from experimenting with EventLoopGroup stuff. Removed

kmahar · 2022-08-18T17:46:38Z

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

+                                )
+                            } catch {
+                                // DB not explicitly initially created for operation-failure test bc no initialData
+                                if !(error is MongoError.CommandError) {


we can make this a little more specific actually, so that we don't ignore arbitrary command errors. the error code for NamespaceNotFound is 26.

let's do something like the logic here so that we only catch and ignore command errors with code 26:

mongo-swift-driver/Tests/MongoSwiftSyncTests/UnifiedTestRunner/UnifiedTestRunner.swift

Line 76 in 2d579e2

} catch let commandError as MongoError.CommandError where commandError.code == 11601 {}

Makes sense. Was struggling with the syntax earlier but this really helps. Done.

kmahar · 2022-08-18T17:50:52Z

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

+                                    ["distinct": .string(entity.name), "key": "_id"]
+                                )
+                            } catch {
+                                // DB not explicitly initially created for operation-failure test bc no initialData


can we make this comment a little mores pecific? something like "this command can fail for tests that have no initialData specified and so do not create the namespace, such as the valid-fail/operation-failure.json test"

Done. Moved comment to before the do/catch block

patrickfreed

Looks really good! Just have a few minor comments

Tests/MongoSwiftTests/UnifiedTestRunner/EntityDescription.swift

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

isabelatkinson

lgtm, just one nit

isabelatkinson · 2022-08-23T17:24:37Z

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift

+#if compiler(>=5.5.2) && canImport(_Concurrency)
+import MongoSwift
 import Nimble
+import NIOPosix


doesn't look like this actually got removed

rchhaya added 26 commits July 26, 2022 14:29

Changing import in UTR.swift and adding makeAsyncClient

ad8e4c8

Changing all imports

102b400

Async execute + try awaits for operations

9fa5a6b

Fully async-ifying and migrating files

c289649

Print debugging close failures

a01c138

Bug on BSON decoder, unbox()

0a4cf01

Fixing failpont/utils errors, need to fix client deinit

fca3858

Remedying package, hanging clients

7887707

Entity clients no longer hang, expected errs still fail

f0219f4

Problem was with configureFailpoint mLaunch, all pass except ursors c…

dec988e

…an be created, iterated, and closed

Failpoints are closed properly, linted

c2625b8

Removing duplicate retrievespectests

2a1cd9a

Compiler directives

4ced5b6

Compiler directives for users of UTR

1de51bf

Failpoint compiler directives

55027fe

Matching compiler directives

bac6885

Function compiler directives

e253340

Compilers

778735d

More compilers for matching.swift:

8e66be5

even more compilers for entities

4469489

Compilers for testfile

05941a6

TestWriteModel compiler

0cc50d8

Undoing error from prior push:

08d04c0

Closing self-created async client

520a2b2

Taking sharding into account

9a57df0

self annotation

f99e356

rchhaya added 3 commits August 10, 2022 09:56

Closing clients upon sharding thrown err

33007c0

Flaky entity-find-cursor, operation-failure

858b7ab

Trying try? to resolve killCursor stuff

5692453

kmahar reviewed Aug 12, 2022

View reviewed changes

rchhaya added 4 commits August 15, 2022 11:15

Reviewing failpoint, comments, renames, methods, tests, etc.

462aba2

Deleting .swift-version

624a764

merge conflicts

079340e

forgotten commented out compiler directive

597e76a

rchhaya requested a review from kmahar August 15, 2022 18:01

Undoing setUp() removal for scoping

82c5163

kmahar reviewed Aug 16, 2022

View reviewed changes

rchhaya added 2 commits August 16, 2022 10:22

Addressing comments-elg,failpoints,comments,deinits

a222b53

Switching order of elg/client shutdown

0d48bf8

rchhaya requested a review from kmahar August 16, 2022 17:54

kmahar reviewed Aug 16, 2022

View reviewed changes

Tests/MongoSwiftTests/Failpoint.swift Outdated Show resolved Hide resolved

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift Outdated Show resolved Hide resolved

Apply suggestions from code review

1c31d13

Co-authored-by: Kaitlin Mahar <kaitlinmahar@gmail.com>

rchhaya requested a review from kmahar August 17, 2022 14:40

rchhaya added 2 commits August 17, 2022 22:17

Removing try? for try to see evergreen results

b887bb2

to try? or not to try?, that is the question

2874160

kmahar reviewed Aug 18, 2022

View reviewed changes

Specifying command error, docs, and removing imports'

46dfa9e

rchhaya requested a review from kmahar August 18, 2022 19:34

kmahar approved these changes Aug 19, 2022

View reviewed changes

patrickfreed reviewed Aug 19, 2022

View reviewed changes

Tests/MongoSwiftTests/UnifiedTestRunner/EntityDescription.swift Outdated Show resolved Hide resolved

Tests/MongoSwiftTests/UnifiedTestRunner/UnifiedTestRunner.swift Outdated Show resolved Hide resolved

Reusing elg for entities, changing print

b7369a5

rchhaya requested a review from patrickfreed August 19, 2022 20:26

isabelatkinson approved these changes Aug 23, 2022

View reviewed changes

patrickfreed added 5 commits August 29, 2022 12:51

remove some unneeded stuff

697b0d7

Merge remote-tracking branch 'origin/main' into SWIFT-1609/utr-async

c134cbd

Merge remote-tracking branch 'origin/main' into SWIFT-1609/utr-async

c55c058

revert to NIOPosix import

0cdf086

sort imports

2d8e274

patrickfreed merged commit 6c26d5e into mongodb:main Aug 30, 2022

SWIFT-1609 Async-ifying the Unified Test Runner and underlying operations #769

SWIFT-1609 Async-ifying the Unified Test Runner and underlying operations #769

Uh oh!

Conversation

rchhaya commented Jul 29, 2022

Uh oh!

rchhaya commented Aug 9, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kmahar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmahar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickfreed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

isabelatkinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants