introduce shared_values module in test_utils which proves the concept of a primitive for out-of-band messaging in tests #2676
Conversation
steveej
force-pushed
the
pr_distributed-test-poc
branch
from
c163af6
to
c8b6b6b
Compare
|
This item has been open for 30 days with no activity. |
github-actions
bot
added
the
stale
steveej
removed
the
stale
steveej
changed the title
steveej
force-pushed
the
pr_distributed-test-poc
branch
from
58c07a0
to
2507d3d
Compare
|
This item has been open for 30 days with no activity. |
github-actions
bot
added
the
stale
the server side can be run e.g. with: ``` env \ TEST_SHARED_VALUES_REMOTEV1_ROLE="server" \ TEST_SHARED_VALUES_REMOTEV1_URL="ws://0.0.0.0:34567" \ cargo nextest run -p holochain_test_utils --no-capture --features slow_tests --status-level=pass distributed ``` clients can then run like this: ``` env \ TEST_SHARED_VALUES_REMOTEV1_ROLE="client" \ TEST_SHARED_VALUES_REMOTEV1_URL="ws://localhost:34567" \ cargo nextest run -p holochain_test_utils --no-capture --features slow_tests --status-level=pass distributed ```
there's still major opportunity to refactor common boilerplate but it's not a priority for this PoC.
steveej
changed the title
steveej
changed the title
| @@ -312,6 +312,13 @@ async fn remote_signals() -> anyhow::Result<()> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| #[ignore = "TODO: part of the next proof of concept"] | |||
There was a problem hiding this comment.
Context required to know what PoC, maybe include a ref to this PR or an issue number?
| version = "0.1.0" | ||
| edition = "2021" | ||
|
|
||
| # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html |
There was a problem hiding this comment.
Please drop generated line, where know where to find this :)
There was a problem hiding this comment.
Prefer src/shared_values.rs referenced from the lib.rs
|
|
||
| use std::{collections::BTreeMap, time::Duration}; | ||
|
|
||
| use anyhow::Result as Fallible; |
There was a problem hiding this comment.
I'd prefer this wasn't renamed, I'm used to looking at Result or SomethingSpecificResult, so this requires adding a translation. If you import anyhow::Result then Result will be used from anyhow rather than std::result::Result
|
|
||
| #[async_trait] | ||
| pub(crate) trait SharedValues: DynClone + Sync + Send { | ||
| async fn put_t(&mut self, key: String, value: String) -> Fallible<Option<String>>; |
There was a problem hiding this comment.
These could use some docs, I can probably guess what the outputs are but those in particular would be useful to have docs for
There was a problem hiding this comment.
Does the _t have a significance beyond distinguishing impl functions from the trait ones? If not I'd prefer that was dropped and the calls were disambiguated where they're made
| tokio::select! { | ||
| data = self.get_pattern(pattern.as_str(), |(_previous_data, data)| { data.len() >= min_data }) => Ok(data?), | ||
| _ = { | ||
| let duration = if let Some(wait_timeout) = maybe_wait_timeout { |
There was a problem hiding this comment.
Something like duration.unwrap_or(std::time::Duration::MAX) but I get those functional methods a little wrong without IDE support :D
|
|
||
| use holochain_websocket::{WebsocketConfig, WebsocketListener}; | ||
|
|
||
| // TODO: deglob this |
There was a problem hiding this comment.
TODO intended for this PR?
| pub(crate) const TEST_AGENT_READINESS_TIMEOUT_SECS_DEFAULT: u64 = 360; | ||
|
|
||
| #[derive(Debug, Default, strum_macros::EnumString)] | ||
| #[strum(serialize_all = "lowercase")] |
There was a problem hiding this comment.
Ahh this is neat, I could do with adding strum to my toolkit then because I usually do this by hand :D
|
|
||
| impl RemoteV1Server { | ||
| /// Creates a new server and starts it immediately. | ||
| pub(crate) async fn new(bind_socket: Option<String>) -> Fallible<Self> { |
There was a problem hiding this comment.
I'd prefer bind_address to bind_socket
| localv1: LocalV1, | ||
| server: &mut WebsocketListener, | ||
| ) -> Fallible<()> { | ||
| while let Some(Ok((/* never sends on its own */ _tx, mut recv))) = server.next().await { |
There was a problem hiding this comment.
is _tx safe to drop though? Because this loop spawns tokio tasks which means this will go out of scope quite quickly
|
This item has been open for 30 days with no activity. |
github-actions
bot
added
the
stale
steveej
force-pushed
the
pr_distributed-test-poc
branch
from
1ffae29
to
8ec2e8e
Compare
steveej
removed
the
stale
this is a step towards a framework for writing
synchronization logic that is suitable for tests that are distributed across many nodes.
the
LocalV1implementation could be removed at this point. i initially relied on it to verify the behavior of theRemoteV1implementation. i don't foresee a scenario where we couldn't use the latter.fixes #3740
TODO: