fix(test): Wait for zebrad and lightwalletd to reach the tip in tests, to improve test coverage #5164

oxarbitrage · 2022-09-14T21:26:10Z

Motivation

We need to make sure lightwalletd and zebrad is at the tip for several tests. Fix #4894 if merged.

Depends-On: #5307

Solution

Add a new function and move common code to check if we are at the tip. Call it from tests. We are calling this function from all tests that call lightwalletd_integration_test and from wallet_grpc_test and send_transaction_test.

Also, the lightwalletd_test_suite now runs wallet_grpc_test before send_transaction_test so we know cache is up to date when running the tests using the suite.

Functional changes:

use the same code to configure, launch, and wait for zebrad and lightwalletd across all 8 RPC integration tests,
this makes all these tests check a bunch of things the same way
run the quick tests first in the test suite, then the slow tests

Refactors:

make some test functions easier to call

Review

This PR is ready for review.

Reviewer Checklist

Will the PR name make sense to users?
- Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
Are the PR labels correct?
Does the code do what the ticket and PR says?
How do you know it works? Does it have tests?
- Do the existing tests run, or are we accidentally skipping them? Check the test time and test logs.

zebrad/tests/common/launch.rs

codecov · 2022-09-14T23:43:10Z

Codecov Report

Merging #5164 (55b6035) into main (9f6a1fd) will increase coverage by 0.07%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5164      +/-   ##
==========================================
+ Coverage   79.09%   79.17%   +0.07%     
==========================================
  Files         308      308              
  Lines       39560    39560              
==========================================
+ Hits        31291    31320      +29     
+ Misses       8269     8240      -29

zebrad/tests/common/launch.rs

teor2345

This is pretty much what we want.

I just want to add a few more checks to some tests, so they run for longer and cover more blocks and code.

zebrad/tests/common/lightwalletd/send_transaction_test.rs

zebrad/tests/common/launch.rs

zebrad/tests/common/lightwalletd/send_transaction_test.rs

zebrad/tests/common/lightwalletd/wallet_grpc_test.rs

zebrad/tests/acceptance.rs

teor2345 · 2022-09-20T05:34:09Z

@Mergifyio update

mergify · 2022-09-20T05:34:33Z

update

✅ Branch has been successfully updated

teor2345 · 2022-09-21T08:23:09Z

I combined the test APIs for the lightwalletd sync and gRPC tests.
This will give us a consistent sync to the tip across all those tests.

But I still need to fix some test failures.

teor2345 · 2022-10-03T20:21:42Z

@Mergifyio update

mergify · 2022-10-03T20:22:19Z

update

✅ Branch has been successfully updated

teor2345 · 2022-10-03T23:05:55Z

Alfredo is going to review this PR, but I can't set him as a reviewer, because he opened it.

oxarbitrage

This looks really good to me. I can't approve as i am the original author but it seems to be merge ready.

upbqdn · 2022-10-04T11:53:58Z

I'm reviewing this PR now.

teor2345

Arya and Alfredo have already reviewed this PR.

gustavovalverde · 2022-10-05T12:24:59Z

@Mergifyio update

mergify · 2022-10-05T12:25:37Z

update

✅ Branch has been successfully updated

arya2 · 2022-10-05T20:39:47Z

mempool_cancel_mined failed in the merge-queue

arya2 · 2022-10-05T20:39:52Z

@Mergifyio refresh

mergify · 2022-10-05T20:40:24Z

refresh

✅ Pull request refreshed

…, to improve test coverage (#5164) * Add RPC timing to zcash-rpc-diff * Use transaction hash index for verbose block requests, rather than block data * check if we are at tip for lightwallet wallet tests * move function * Apply suggestions from code review Co-authored-by: teor <teor@riseup.net> * Combine the lightwalletd sync and gRPC test APIs * Rewrite the gRPC and full sync tests for the new APIs * Make zebra_rpc_address optional because only some tests need it * Check for the zebrad RPC port to open in the right place * Do the quick lightwalletd integration tests first in the sequential test function * Ignore the lightwalletd cached state env var in tests that don't want it * Don't replace the state path in RPC tests * Enable IO (and timers) on the tip check tokio runtime * Stop waiting for sync if either waiter thread errors or panics * Try to speed up slow lightwalletd full syncs * Don't wait for the tip in send transaction tests, and try to speed up full lightwalletd syncs * Remove redundant is_lightwalletd_finished store Co-authored-by: Arya <aryasolhi@gmail.com> * Fix unused variable error * Actually create the lightwalletd cached state * Fix lwd cache check logic Co-authored-by: teor <teor@riseup.net> Co-authored-by: Arya <aryasolhi@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Previous behavior: In PR #5164, we made lightwalletd sync all the way to the tip in its full sync test. This increases that test's time from 1 hour to 4 hours, which makes the CI we run on each PR change increase from 3 hours to 6 hours. Expected behavior: Run the lightwalletd full sync just on `main` or if a state disk for the actual version is not found. Solution: Add the `github.event_name == 'push' && github.ref_name == 'main'` condition to the `lightwalletd-full-sync` test. Fixes #5316

…5393) * ci(sync): only run the `lightwalletd` full sync on the `main` branch Previous behavior: In PR #5164, we made lightwalletd sync all the way to the tip in its full sync test. This increases that test's time from 1 hour to 4 hours, which makes the CI we run on each PR change increase from 3 hours to 6 hours. Expected behavior: Run the lightwalletd full sync just on `main` or if a state disk for the actual version is not found. Solution: Add the `github.event_name == 'push' && github.ref_name == 'main'` condition to the `lightwalletd-full-sync` test. Fixes #5316 * Allow lwd full syncs to be triggered manually (#5400) * Limit checkpoint and lwd full sync concurrency * Add a patch job for lightwalletd-full-sync Co-authored-by: teor <teor@riseup.net>

teor2345 assigned arya2 and teor2345 Sep 14, 2022

teor2345 reviewed Sep 14, 2022

View reviewed changes

zebrad/tests/common/launch.rs Outdated Show resolved Hide resolved

teor2345 reviewed Sep 14, 2022

View reviewed changes

zebrad/tests/common/launch.rs Outdated Show resolved Hide resolved

teor2345 reviewed Sep 14, 2022

View reviewed changes

zebrad/tests/common/launch.rs Outdated Show resolved Hide resolved

teor2345 reviewed Sep 14, 2022

View reviewed changes

teor2345 unassigned arya2 Sep 14, 2022

teor2345 requested review from arya2 and removed request for arya2 September 14, 2022 23:54

teor2345 added A-rust Area: Updates to Rust code P-Medium ⚡ C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces labels Sep 14, 2022

teor2345 changed the title ~~fix(test): Wait zebrad and lightwalletd to reach the tip in some tests~~ fix(test): Wait for zebrad and lightwalletd to reach the tip in some tests Sep 15, 2022

teor2345 added C-enhancement Category: This is an improvement S-incomplete and removed S-incomplete labels Sep 20, 2022

teor2345 mentioned this pull request Sep 20, 2022

doc(ci): Fix incorrect docs about Docker build cache order #5190

Merged

1 task

teor2345 force-pushed the issue4894 branch from c6e5f50 to 335099c Compare September 21, 2022 08:21

teor2345 force-pushed the issue4894 branch from 335099c to 4f67c2f Compare September 23, 2022 01:50

teor2345 changed the title ~~fix(test): Wait for zebrad and lightwalletd to reach the tip in some tests~~ fix(test): Wait for zebrad and lightwalletd to reach the tip in tests, to improve test coverage Sep 23, 2022

teor2345 marked this pull request as ready for review September 23, 2022 02:20

teor2345 requested a review from a team as a code owner September 23, 2022 02:20

This comment was marked as resolved.

Sign in to view

teor2345 removed the request for review from a team September 23, 2022 02:21

Merge branch 'main' into issue4894

21ce4b7

This comment was marked as resolved.

Sign in to view

teor2345 removed the request for review from upbqdn October 3, 2022 23:05

teor2345 mentioned this pull request Oct 4, 2022

fix(test): Add a timeout to the non_blocking_logger test #5325

Merged

5 tasks

This comment was marked as resolved.

Sign in to view

oxarbitrage commented Oct 4, 2022

View reviewed changes

teor2345 approved these changes Oct 4, 2022

View reviewed changes

mergify bot added a commit that referenced this pull request Oct 4, 2022

Merge of #5164

48ae83a

mergify bot mentioned this pull request Oct 4, 2022

merge-queue: embarking main (a418985) and #5164 together #5333

Closed

Merge branch 'main' into issue4894

55b6035

mergify bot added a commit that referenced this pull request Oct 5, 2022

Merge of #5164

3dba11d

mergify bot mentioned this pull request Oct 5, 2022

merge-queue: embarking main (fd63d33) and #5164 together #5344

Closed

mergify bot added a commit that referenced this pull request Oct 5, 2022

Merge of #5164

c0cd1fd

mergify bot mentioned this pull request Oct 5, 2022

merge-queue: embarking main (8a5708c) and #5164 together #5346

Closed

mergify bot merged commit 1937b6c into main Oct 6, 2022

mergify bot deleted the issue4894 branch October 6, 2022 04:12

teor2345 mentioned this pull request Oct 11, 2022

Release Zebra 1.0.0-rc.0 #5383

Merged

32 tasks

gustavovalverde mentioned this pull request Oct 12, 2022

ci(sync): only run the lightwalletd full sync on the main branch #5393

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(test): Wait for zebrad and lightwalletd to reach the tip in tests, to improve test coverage #5164

fix(test): Wait for zebrad and lightwalletd to reach the tip in tests, to improve test coverage #5164

oxarbitrage commented Sep 14, 2022 •

edited by teor2345

codecov bot commented Sep 14, 2022 •

edited

teor2345 left a comment

teor2345 commented Sep 20, 2022

mergify bot commented Sep 20, 2022

teor2345 commented Sep 21, 2022

This comment was marked as resolved.

teor2345 commented Oct 3, 2022

mergify bot commented Oct 3, 2022

This comment was marked as resolved.

teor2345 commented Oct 3, 2022

This comment was marked as resolved.

oxarbitrage left a comment

upbqdn commented Oct 4, 2022

teor2345 left a comment

gustavovalverde commented Oct 5, 2022

mergify bot commented Oct 5, 2022

arya2 commented Oct 5, 2022

arya2 commented Oct 5, 2022

mergify bot commented Oct 5, 2022

fix(test): Wait for zebrad and lightwalletd to reach the tip in tests, to improve test coverage #5164

fix(test): Wait for zebrad and lightwalletd to reach the tip in tests, to improve test coverage #5164

Conversation

oxarbitrage commented Sep 14, 2022 • edited by teor2345

Motivation

Solution

Review

Reviewer Checklist

codecov bot commented Sep 14, 2022 • edited

Codecov Report

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 commented Sep 20, 2022

mergify bot commented Sep 20, 2022

✅ Branch has been successfully updated

teor2345 commented Sep 21, 2022

This comment was marked as resolved.

teor2345 commented Oct 3, 2022

mergify bot commented Oct 3, 2022

✅ Branch has been successfully updated

This comment was marked as resolved.

teor2345 commented Oct 3, 2022

This comment was marked as resolved.

oxarbitrage left a comment

Choose a reason for hiding this comment

upbqdn commented Oct 4, 2022

teor2345 left a comment

Choose a reason for hiding this comment

gustavovalverde commented Oct 5, 2022

mergify bot commented Oct 5, 2022

✅ Branch has been successfully updated

arya2 commented Oct 5, 2022

arya2 commented Oct 5, 2022

mergify bot commented Oct 5, 2022

✅ Pull request refreshed

oxarbitrage commented Sep 14, 2022 •

edited by teor2345

codecov bot commented Sep 14, 2022 •

edited