Investigate flaky tests #74
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Should have done this in b69a40d.
As required by the accompanying ably-cocoa submodule bump.
As required by the accompanying ably-cocoa submodule bump (see the ably-cocoa commit message for explanation of why we're making this change).
We have not yet implemented this and will not be doing so before our initial release. Also the API is not fully correct — as things stand, if you extract a LiveObject from a map inside the batch callback (see first example on [1]) you have to use an async API to interact with it, which is not what we want. [1] https://ably.com/docs/liveobjects/batch
This allows us to use "run repeatedly" without the runs interfering with each other. wip adding test UUID to logs this isn't quite right — we need to be able to do client2 and have the ID there too (probably should bake the test into the context) also Test overlaps with Swift Testing terminology now
- number of concurrent Realtime instances - when monitorConnectionThenCloseAndFinishAsync fails (to see whether it's actually failing the test when it should be)
asked Cursor to write this trait that limits concurrency this is because there seem to be issues with running concurrent tests (hangs, and failures that I think are because of it exercising edge cases in the tests, but also Realtime's "Error 40111 - account restricted (connection limit exceeded)" and connection timeouts) my logging suggests that we have up to 100 Realtime instances active at a time I was going to just make things .serialized for now but that makes the test suite really slow (like 4 minutes) so I hoped we could find a middle ground I tried making it a suite trait that has isRecursive: true (which I thought was the default which is why I made those earlier changes to the fixtures trait) but it doesn't seem to be. I've however observed this hanging at least once locally so won't be taking this forward for now (need to get the test suite fixed and have a time pressure)
Mistake in 70306a0. Correct approach copied from "Objects.createCounter sends COUNTER_CREATE operation" test.
Wait until we're sure that GC of an entry should have occurred, regardless of clock skew. Also fix what may have been a bug in 792683d where we may not wait for as many GC cycles as we intended to (because of counting AsyncStream buffered values).
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to #72. some added logging and attempts and limiting concurrency, had to be parked for now due to time