Skip to content

Backport RPC test harness PRs #4411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
zkbot merged 4 commits into from
Apr 3, 2020
Merged

Backport RPC test harness PRs #4411

zkbot merged 4 commits into from
Apr 3, 2020

Conversation

@str4d
Copy link
Contributor

@str4d str4d commented Mar 24, 2020 •
edited
Loading

@ptschip @str4d
Enable python tests for Native Windows
8f863b2 
1) Multiplatorm support for devnull
2) Fixed a bug in the handling of cache files
3) Deleted run-bitcoin-cli as no longer needed
@str4d str4d added A-testing Area: Tests and testing infrastructure C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. labels Mar 24, 2020
@str4d
Copy link
Contributor Author

str4d commented Mar 24, 2020

@zkbot try

@zkbot
Copy link
Contributor

zkbot commented Mar 24, 2020

⌛ Trying commit 73fc95918ce3f2fcb6738f82e29f9676c7049ea5 with merge b2ab91b0326c9082a32dd3d1121f482c764d46e2...

@str4d str4d mentioned this pull request Mar 24, 2020
Merged
@zkbot
Copy link
Contributor

zkbot commented Mar 24, 2020

💔 Test failed - pr-try

jamesob and others added 3 commits March 24, 2020 21:51
@jamesob @str4d
Add basic coverage reporting for RPC tests
d38a012 
Thanks to @MarcoFalke @dexX7 @laanwj for review.

Zcash: Includes all the backend changes, but excludes the --coverage flag
added to rpc-tests.py as we have not yet migrated away from rpc-tests.sh.
@laanwj @str4d
test_framework: detect failure of bitcoind startup
11240d0 
Replace the `bitcoin-cli -rpcwait` after spawning bitcoind
with our own loop that detects when bitcoind exits prematurely.

And if one node fails to start, stop the others.

This prevents a hang in such a case (see #7463).
@NicolasDorier @str4d
[qa] assert_start_raises_init_error
b0d81be 
Zcash: Excludes wallet-hd.py change (missing bitcoin/bitcoin#8309)
@str4d str4d force-pushed the 2074-rpc-test-harness-1 branch from 73fc959 to b0d81be Compare March 24, 2020 08:59
@str4d
Copy link
Contributor Author

str4d commented Mar 24, 2020

Fixed some pyflakes-detected bugs; I tweaked the individual commits to address them. I also ran a couple of the RPC tests locally to check that they passed.

@zkbot try

@zkbot
Copy link
Contributor

zkbot commented Mar 24, 2020

⌛ Trying commit b0d81be with merge 38755eb...

zkbot added a commit that referenced this pull request Mar 24, 2020
@zkbot
Auto merge of #4411 - str4d:2074-rpc-test-harness-1, r=<try>
38755eb 
Backport RPC test harness PRs

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6548
- bitcoin/bitcoin#6804
  - Just the coverage backend, not the flag to enable it.
- bitcoin/bitcoin#7744
- bitcoin/bitcoin#9832
  - Excludes `wallet-hd.py` change (missing bitcoin/bitcoin#8309).

Part of #2074.
@zkbot
Copy link
Contributor

zkbot commented Mar 24, 2020

☀️ Test successful - pr-try
State: approved= try=True

@mdr0id
Copy link
Contributor

mdr0id commented Mar 28, 2020

@rex4539 Is it possible to check this PR on your Mac when you get a cycle? It would be very much appreciated.

Otherwise, so far looks good modulo Windows. The only thing that might cause an issue was dev/null paths, but this PR appears to remove them anyway.

@rex4539
Copy link
Contributor

rex4539 commented Mar 29, 2020 

Tests completed: 73
successes 73; failures: 0

@str4d
Copy link
Contributor Author

str4d commented Mar 29, 2020

@mdr0id yep, the backported PRs included the change that you had already made for Windows support, as well as a subsequent change that removes them entirely.

@mdr0id mdr0id self-requested a review March 30, 2020 16:44
mdr0id
mdr0id approved these changes Mar 30, 2020
View reviewed changes
Copy link
Contributor

@mdr0id mdr0id left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@mdr0id
Copy link
Contributor

mdr0id commented Apr 2, 2020

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Apr 2, 2020

📌 Commit b0d81be has been approved by mdr0id

@zkbot
Copy link
Contributor

zkbot commented Apr 2, 2020

⌛ Testing commit b0d81be with merge da12da80c4d781579ed7e767f4ad24620a84e98c...

@str4d str4d added this to the Core Sprint 2020-13 milestone Apr 3, 2020
@str4d str4d added the S-scratching-an-itch Status: Something we haven't planned for a sprint but are doing anyway label Apr 3, 2020
@zkbot
Copy link
Contributor

zkbot commented Apr 3, 2020

💔 Test failed - pr-merge

@str4d
Copy link
Contributor Author

str4d commented Apr 3, 2020

Looks like a transient failure.

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented Apr 3, 2020

⌛ Testing commit b0d81be with merge 1be7250...

@zkbot
Copy link
Contributor

zkbot commented Apr 3, 2020

☀️ Test successful - pr-merge
Approved by: mdr0id
Pushing 1be7250 to master...

@zkbot zkbot merged commit 1be7250 into zcash:master Apr 3, 2020
@str4d str4d deleted the 2074-rpc-test-harness-1 branch April 3, 2020 08:10
str4d pushed a commit to str4d/zcash that referenced this pull request Nov 21, 2020
@jamesob @str4d
Add basic coverage reporting for RPC tests
eaf4dfe 
Thanks to @MarcoFalke @dexX7 @laanwj for review.

Zcash: Just the --coverage flag for rpc-tests.py, as we backported the
rest of the coverage backend in zcash#4411.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mdr0id mdr0id mdr0id approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-testing Area: Tests and testing infrastructure C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. S-scratching-an-itch Status: Something we haven't planned for a sprint but are doing anyway

Projects

None yet

Milestone

Core Sprint 2020-13

Development

Successfully merging this pull request may close these issues.

8 participants

@str4d @zkbot @mdr0id @rex4539 @ptschip @jamesob @laanwj @NicolasDorier