From a63fe225a06a761bc4dbb767f88b4da9dd1c987a Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 13 Sep 2023 00:33:09 -0300 Subject: [PATCH] change(tests): Obtain lightwalletd tip from logs instead of grpc (#7507) * obtain lightwalletd from the logs * revert #7332 * temporarily stop checking that the update sync has a good cached state * Revert "temporarily stop checking that the update sync has a good cached state" This reverts commit 0fb10938d9903d59c338b93a6fad4911aa2d969e. * add `save_to_disk` to workflow * Fix lightwalletd height_grep_text for ECC fork * Add a TODO about blocking the async executor --------- Co-authored-by: teor --- .../continous-integration-docker.yml | 8 ++-- zebrad/tests/acceptance.rs | 2 +- zebrad/tests/common/lightwalletd/sync.rs | 43 +++++++++++++------ 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/.github/workflows/continous-integration-docker.yml b/.github/workflows/continous-integration-docker.yml index 58907fff..03324e7c 100644 --- a/.github/workflows/continous-integration-docker.yml +++ b/.github/workflows/continous-integration-docker.yml @@ -606,7 +606,7 @@ jobs: root_state_path: '/var/cache' zebra_state_dir: 'zebrad-cache' lwd_state_dir: 'lwd-cache' - height_grep_text: 'Adding block to cache ' + height_grep_text: 'Waiting for block: ' secrets: inherit # We want to prevent multiple lightwalletd full syncs running at the same time, # but we don't want to cancel running syncs on `main` if a new PR gets merged, @@ -636,14 +636,14 @@ jobs: test_variables: '-e NETWORK=${{ inputs.network || vars.ZCASH_NETWORK }} -e TEST_LWD_UPDATE_SYNC=1 -e ZEBRA_TEST_LIGHTWALLETD=1 -e ZEBRA_FORCE_USE_COLOR=1 -e ZEBRA_CACHED_STATE_DIR=/var/cache/zebrad-cache -e LIGHTWALLETD_DATA_DIR=/var/cache/lwd-cache' needs_zebra_state: true needs_lwd_state: true - # since we do a full sync in every PR, the new cached state will only be a few minutes newer than the original one - saves_to_disk: false + saves_to_disk: true + force_save_to_disk: ${{ inputs.force_save_to_disk || false }} disk_prefix: lwd-cache disk_suffix: tip root_state_path: '/var/cache' zebra_state_dir: 'zebrad-cache' lwd_state_dir: 'lwd-cache' - height_grep_text: 'Adding block to cache ' + height_grep_text: 'Waiting for block: ' secrets: inherit # Test that Zebra can answer a synthetic RPC call, using a cached Zebra tip state diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 0e6356b9..17abe47d 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1835,7 +1835,7 @@ fn lightwalletd_integration_test(test_type: TestType) -> Result<()> { if test_type.needs_lightwalletd_cached_state() { lightwalletd - .expect_stdout_line_matches("Done reading [0-9]{1,7} blocks from disk cache")?; + .expect_stdout_line_matches("Done reading [0-9]{7} blocks from disk cache")?; } else if !test_type.allow_lightwalletd_cached_state() { // Timeout the test if we're somehow accidentally using a cached state in our temp dir lightwalletd.expect_stdout_line_matches("Done reading 0 blocks from disk cache")?; diff --git a/zebrad/tests/common/lightwalletd/sync.rs b/zebrad/tests/common/lightwalletd/sync.rs index a44a76f7..8dce05a9 100644 --- a/zebrad/tests/common/lightwalletd/sync.rs +++ b/zebrad/tests/common/lightwalletd/sync.rs @@ -11,11 +11,7 @@ use tempfile::TempDir; use zebra_node_services::rpc_client::RpcRequestClient; use zebra_test::prelude::*; -use crate::common::{ - launch::ZebradTestDirExt, - lightwalletd::wallet_grpc::{connect_to_lightwalletd, ChainSpec}, - test_type::TestType, -}; +use crate::common::{launch::ZebradTestDirExt, test_type::TestType}; /// The amount of time we wait between each tip check. pub const TIP_CHECK_RATE_LIMIT: Duration = Duration::from_secs(60); @@ -110,7 +106,7 @@ pub fn wait_for_zebrad_and_lightwalletd_sync< } tracing::info!(?test_type, "waiting for lightwalletd to sync to the tip..."); - while !are_zebrad_and_lightwalletd_tips_synced(lightwalletd_rpc_port, zebra_rpc_address)? { + while !are_zebrad_and_lightwalletd_tips_synced(zebra_rpc_address, lightwalletd_mut)? { let previous_check = Instant::now(); // To improve performance, only check the tips occasionally @@ -171,22 +167,41 @@ pub fn wait_for_zebrad_and_lightwalletd_sync< /// Returns `Ok(true)` if zebrad and lightwalletd are both at the same height. #[tracing::instrument] pub fn are_zebrad_and_lightwalletd_tips_synced( - lightwalletd_rpc_port: u16, zebra_rpc_address: SocketAddr, + lightwalletd: &mut TestChild, ) -> Result { let rt = tokio::runtime::Builder::new_multi_thread() .enable_all() .build()?; rt.block_on(async { - let mut lightwalletd_grpc_client = connect_to_lightwalletd(lightwalletd_rpc_port).await?; + // We are going to try getting the current lightwalletd height by reading the lightwalletd logs. + let mut lightwalletd_next_height = 1; - // Get the block tip from lightwalletd - let lightwalletd_tip_block = lightwalletd_grpc_client - .get_latest_block(ChainSpec {}) - .await? - .into_inner(); - let lightwalletd_tip_height = lightwalletd_tip_block.height; + // Only go forward on getting next height from lightwalletd logs if we find the line we are interested in. + // + // TODO: move this blocking code out of the async executor. + // The executor could block all tasks and futures while this code is running. + // That's ok for now, but it might cause test hangs or failures if we spawn tasks, select(), or join(). + if let Ok(line) = lightwalletd.expect_stdout_line_matches("Waiting for block: [0-9]+") { + let line_json: serde_json::Value = serde_json::from_str(line.as_str()) + .expect("captured lightwalletd logs are always valid json"); + let msg = line_json["msg"] + .as_str() + .expect("`msg` field is always a valid string"); + + // Block number is the last word of the message. We rely on that specific for this to work. + let last = msg + .split(' ') + .last() + .expect("always possible to get the last word of a separated by space string"); + lightwalletd_next_height = last + .parse() + .expect("the last word is always the block number so it can be parsed to i32 "); + } + + // The last height in lightwalletd is the one the program is expecting minus one. + let lightwalletd_tip_height = (lightwalletd_next_height - 1) as u64; // Get the block tip from zebrad let client = RpcRequestClient::new(zebra_rpc_address);