Await onClose callback in testTransactionsCallbacksOnDestruct

[webpatser]

Follow-up from #144.

@trowski you mentioned in #144 that testTransactionsCallbacksOnDestruct should be independent of the server. Digging in: the test is environment-independent in intent, but it relies on delay(0) being long enough for the destructor's async ROLLBACK round-trip to complete. That's a one-tick race, not a deterministic wait, and it's the reason the MariaDB job was tripping.

Reproduction

Against MariaDB 12.2.2 via Docker on macOS:

1) Amp\Mysql\Test\MysqlConnectionTest::testTransactionsCallbacksOnDestruct
Expectation failed for method name is "__invoke" when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.

Against MySQL 8.4.9 via Docker on macOS the same test also fails. With instrumentation, onRollback and onClose fire 50-80 event loop ticks after delay(0) returns on both servers. The test passes in CI only because localhost MySQL on the GitHub Actions runner wins the race within a single tick.

So both your read and mine were right: the test is server-independent in spirit, and flaky on every server in practice. Just less visibly on the CI box.

Fix

Replace delay(0) with awaiting a DeferredFuture completed by an onClose callback. No timeout, no tick counting, deterministic on either server.

Verification

MysqlConnectionTest: 62/62 passing against MariaDB 12.2.2 and MySQL 8.4.9, both via Docker.

Not a hard sell. If you'd rather keep the delay(0) form and document this as a known-flaky case, I won't push back.

[kelunik]

We obviously don't want to keep a flaky test, thanks!

[trowski]

Thanks @webpatser! I guess server-independent wasn't really correct — rather that I didn't see a reason the test would fail with MariaDB if it passes with MySQL. Having the test be flaky was certainly not my intention.

As a heads up, we typically add a TimeoutCancellation to a Future::await() call in a test to prevent the test hanging indefinitely if the action isn't performed as expected: 3528c68. No doubt you'd be able to find examples where we forgot this ourselves. 😊

[webpatser]

Good catch @trowski, you're right. The await() I added has no TimeoutCancellation, so if onClose never fires the test hangs instead of failing cleanly, which defeats half the point of moving off delay(0).

I'll send a quick follow-up wrapping it with a TimeoutCancellation, matching the pattern in 3528c68. One-liner.

[webpatser]

Ah, I see it now @trowski, 3528c68 already wraps that exact await() with TimeoutCancellation(1). You beat me to it, so scratch the follow-up. Thanks for the pattern pointer.