[test_loop] Convert current tests to use TestLoopV2 #11524
Merged
Conversation
This file contains 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
added 5 commits
June 7, 2024 23:12
…f<Client> + AsRef<AccountId>
…essages across clients
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11524 +/- ##
==========================================
- Coverage 71.24% 71.20% -0.05%
==========================================
Files 794 794
Lines 160941 160944 +3
Branches 160941 160944 +3
==========================================
- Hits 114670 114598 -72
- Misses 41289 41361 +72
- Partials 4982 4985 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
shreyan-gupta
commented
Jun 9, 2024
| Duration::seconds(EPOCH_LENGTH as i64), | ||
| ); | ||
|
|
||
| // Check that the balances are correct. | ||
| let clients = node_datas |
There was a problem hiding this comment.
Note: We are using the change from AsRef to Borrow semantics here.
This was referenced Jun 10, 2024
Merged
shreyan-gupta
requested review from
tayfunelmas,
pugachAG,
Longarithm,
staffik and
robin-near
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 11, 2024
…11520) Part 2 Change in interface of ClientQueries to not depend on `AsRef<AccountId>`. Note that we can get the account_id from Client using the validator signer module. ### Other PRs [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2
… of AsRef<Client> + AsRef<AccountId>" This reverts commit 8c33367.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 12, 2024
Part 1 This PR has the set of major test loop infrastructure changes ## data.rs This is the new implementation of storage of data on test loop. The way it works is, we store any arbit data in a vec of type Any data and return a handle for the data stored. The handle is a lightweight "pointer" to the data (basically stores the index) that can be easily passed around. This effectively acts like a smart pointer to access specific data. We also have the ability to register actors which create a "sender" out of the actor handler and return it. Why was this needed? For managing ownership of data like client_actor, shards_manager_actor etc. We need to create "senders" for our actor handlers and they need to be lightweight, cloneable, implement Send + Sync for passing around threads etc. Alternatives? The most obvious one that I had tried first was to use Arc<Mutex<Actor>> but that was causing some concurrency related issues and wasn't an elegant solution. Additionally, only the senders then had access to the actor data which wasn't easily scalable for things like adhoc events etc. ## sender.rs TestLoopSender is a nice wrapper on top of actors that implements the `CanSend<Msg>` interface. This pattern is similar to what we have for actix wrapper (core/async/src/actix_wrapper.rs) for actors. TestLoopSender needs to implement `DelayedActionRunner` as well for things to function. We don't need to go too much into details of what this is, but it's effectively used by the actor to schedule events sometime in the future. Link to old TestLoopSender PR: #11450 ## futures.rs Newer implementation of future execution for TestLoopV2 using callbacks as the main event. The overall logic for how things work is still the same. I had to do some playing around with FutureSpawner but nothing much has changed functionality and implementation wise. ## test_loop.rs This file implements TestLoopV2 - Got rid of the builder as we don't need it. - "Events" are now of only one type, the CallbackEvent - `future_spawner`, `async_computation_spawner` are exposed by the testloop that can be passed to actors. - `register_actor` to store actor to test_loop_data and returns sender. ## Other PRs [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2 [Part 8](#11539): [test_loop] Simple TestLoopEnvBuilder
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 12, 2024
…11522) Part 3 This PR has the implementation of sync_actor_maker function for test loop. We don't have control over when the sync_actor_maker function is called during execution so we use an adhoc callback event to register the newly created sync_actor with testloop. ### Other PRs [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2 [Part 8](#11539): [test_loop] Simple TestLoopEnvBuilder
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 13, 2024
…essages across clients (#11523) Part 4 This PR adds TestLoopPeerManagerActor for managing network messages. Note that for handling messages between clients, the module needs to have full details of all clients to route the message to. The way TestLoopPeerManagerActor works is by registering a set of handler functions that direct the messages to specific actors. TestLoopPeerManagerActor comes with a set of default handlers, one for client, partial_witness, and shards_manager. On top of these default handlers, we have the ability to register override handlers which are tried before the default handlers. The override handlers can do a bunch of things like modify requests or drop requests. Note that each client/node gets ONE SEPARATE implementation of TestLoopPeerManagerActor. For a 4 client setup, we have 4 instances of the peer manager actor. Each can have their own separate overrides. ### Other PRs [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2 [Part 8](#11539): [test_loop] Simple TestLoopEnvBuilder
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 13, 2024
Part 6 🎉🎉🎉🎉🎉 Nothing more staisfying than seeing this  ### Other PRs [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2 [Part 8](#11539): [test_loop] Simple TestLoopEnvBuilder
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 13, 2024
Part 7 Turns out for the visualizer tool to work, we require the "description" emitted in `EventStartLogOutput` to be of a very specific format `(node_id, Title(Subtitle(...)))` In this PR, we try to add these additional details during logging. This isn't as great as what we originally had as we don't pretty print the individual event types etc. but still should be good enough for debugging. We can further improve this later as per necessity. ### Other PRs [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2 [Part 8](#11539): [test_loop] Simple TestLoopEnvBuilder
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 13, 2024
Part 8 Reduce code duplication by creating a simple test loop environment builder. Over time we can add more functionality to this as per requirements. [Part 1](#11521): [test_loop] Introduce TestLoopV2 [Part 2](#11520): [test_loop] Change ClientQueries trait requirements in TestLoop util [Part 3](#11522): [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2 [Part 4](#11523): [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients [Part 5](#11524): [test_loop] Convert current tests to use TestLoopV2 [Part 6](#11525): [test_loop] Cleanup old test loop code [Part 7](#11528): [test_loop] Better visualizer support for TestLoopV2 [Part 8](#11539): [test_loop] Simple TestLoopEnvBuilder
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.
Part 5
🎉🎉🎉🎉🎉
We can follow this up with a test loop builder to avoid duplicating logic.
Other PRs
Part 1: [test_loop] Introduce TestLoopV2
Part 2: [test_loop] Change ClientQueries trait requirements in TestLoop util
Part 3: [test_loop] test_loop_sync_actor_maker implementation in TestLoopV2
Part 4: [test_loop] Introduce TestLoopPeerManagerActor for handling network messages across clients
Part 5: [test_loop] Convert current tests to use TestLoopV2
Part 6: [test_loop] Cleanup old test loop code
Part 7: [test_loop] Better visualizer support for TestLoopV2
Part 8: [test_loop] Simple TestLoopEnvBuilder