Reshare e2e test #1254
Reshare e2e test #1254
Conversation
There was a problem hiding this comment.
🌟 This is great as i am really depressed about the fact that our CI is still not reliably passing after all the things we have tried.
I could only get this to work locally by manually adding the line:
reshare-test=["entropy-runtime/reshare-test"]to ./node/cli/Cargo.toml. So please either add that or document how to run the test without needing to add that.
Also, i am not so sure about the second part of the test where we wait 80 seconds for another reshare. I ran the test 3 times locally, and twice is passed but once it failed with:
---- validator::tests::test_reshare_e2e stdout ----
thread 'validator::tests::test_reshare_e2e' panicked at crates/threshold-signature-server/src/validator/tests.rs:297:5:
assertion `left != right` failed
Yes, probably this is because i have slow hardware. But i think ideally the second reshare test should use the same code as the first bit, where we first wait to see if signers have changed, then wait another 10s for the rotation.
| @@ -5,6 +5,9 @@ tagged as the final release. | |||
|
|
|||
| ## Pre-Prep | |||
| - [ ] Inform relevant parties that you're preparing a release (e.g, by posting on Discord) | |||
| - [ ] Manually run e2e reshare test | |||
| (first compile node with features=reshare-test and then run | |||
There was a problem hiding this comment.
I took this to mean do this:
cargo build -p entropy --release --features reshare-test
but this gives me:
error: none of the selected packages contains these features: reshare-test
How are you compiling the runtime with this feature flag without having it present in node/cli/Cargo.toml and propagated down?
There was a problem hiding this comment.
gonna add based on the other comment, but weird I don't see it locally or on CI
hmmmm interesting worked locally and on the CI, but will add |
Co-authored-by: peg <[email protected]>
| @@ -5,6 +5,9 @@ tagged as the final release. | |||
|
|
|||
| ## Pre-Prep | |||
| - [ ] Inform relevant parties that you're preparing a release (e.g, by posting on Discord) | |||
| - [ ] Manually run e2e reshare test | |||
There was a problem hiding this comment.
I would try and avoid running this manually and instead make it a part of the release CI job. We have an on > push > release/... stage where we can add an extra testing stage which encompasses this.
There was a problem hiding this comment.
should be doable but let me do in another PR so I can work with zach of vi on that
| pub const EPOCH_DURATION_IN_BLOCKS: BlockNumber = 4 * HOURS; | ||
| #[cfg(feature = "reshare-test")] | ||
| pub const EPOCH_DURATION_IN_BLOCKS: BlockNumber = 15 / (SECS_PER_BLOCK as BlockNumber); |
There was a problem hiding this comment.
Why define this in terms of SECS_PER_BLOCK instead of having something fixed for tests? Say, 1 * MINUTE or something like that?
There was a problem hiding this comment.
because I want it less than one minuet and I I get an overflow problem that way
| @@ -113,4 +113,5 @@ alice =[] | |||
| bob =[] | |||
| # Enable this feature to run the integration tests for the wasm API of entropy-protocol | |||
| # This requires the entropy-protocol node-js module to be built and so is not run by default | |||
| wasm_test=[] | |||
| wasm_test =[] | |||
| reshare-test=[] | |||
There was a problem hiding this comment.
We should add some comments explaining the feature here and in the other TOML files
| let (_validator_ips, _validator_ids) = | ||
| spawn_testing_validators(crate::helpers::tests::ChainSpecType::IntegrationJumpStarted) | ||
| .await; | ||
|
|
||
| let force_authoring = true; | ||
| let context = | ||
| test_node_process_testing_state(ChainSpecType::IntegrationJumpStarted, force_authoring) | ||
| .await; | ||
| let api = get_api(&context[0].ws_url).await.unwrap(); | ||
| let rpc = get_rpc(&context[0].ws_url).await.unwrap(); | ||
| let client = reqwest::Client::new(); |
There was a problem hiding this comment.
We can replace this with spawn_tss_nodes_and_start_chain()
Co-authored-by: Hernando Castano <[email protected]>
Adds a feature flag
reshare-testto shorten session time and allow us to e2e test reshare test.I added a step to release to test this manually, I don't think this is needed to be tested in CI everytime since it would be a whole recompile and would take time, however I am open to suggestions here
Closes #1210