Skip to content
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.

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

feat(tests): Move the RPC tests framework from zcashd #8866

Merged
merged 16 commits into from
Sep 20, 2024

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Sep 16, 2024

Motivation

We want to integrate zcashd's RPC test framework with Zebra to ensure compatibility. To achieve this, we are migrating the framework and tests from zcashd to Zebra. Each test migration requires specific changes to Zebra, and this PR begins the process by moving the test framework and a few initial test.

This PR addresses parts of #8779 and #8781

Solution

This PR do the the groundwork for the migration, but it is not the complete solution.

  • README was copied from https://github.com/zcash/zcash/blob/master/qa/README.md and updated.
  • The RPC test runner script, rpc-tests.py, was copied and modified where necessary to adapt it for Zebra. This script runs with every pull request in zcashd’s CI.
  • The rpc-tests framework directory was migrated, though most files remain unchanged. The following modifications were made:
    • Updated the util.py file as required.
    • Introduced a new proxy.py file, based on authproxy.py, with authentication removed.
  • The getmininginfo.py test was moved without modification, as the first test in this migration.
  • The reindex test was moved with modifications, to match zebrad sync behavior.
  • create_cache was moved so the new framework can create a cache state to speed up tests.

Tests

Run the framework:

 ./qa/pull-tester/rpc-tests.py

Individual tests:

./qa/pull-tester/rpc-tests.py getmininginfo

Direct:

./qa/rpc-tests/getmininginfo.py

Follow-up Work

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@github-actions github-actions bot added C-feature Category: New features C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Sep 16, 2024
@oxarbitrage
Copy link
Contributor Author

A lot of the files in this PR are moved from zcashd unmodified, almost everything inside qa/rpc-tests/test-framework was not modified. I made a small script so reviewers can check the files were moved untouched. It checks the differences between several files in zcashd and the files in this PR:

#!/bin/bash

# Array of files to compare
files=("__init__.py" "authproxy.py" "bignum.py" "blockstore.py" "blocktools.py" "compool.py" "coverage.py" "equihash.py" "flyclient.py" "key.py" "mininode.py" "netutil.py" "script.py" "script.py" "socks5.py" "test_framework.py" "util.py" "zip244.py" "zip317.py")

# Base URLs for Zebra and Zcash
zebra_base="https://raw.githubusercontent.com/ZcashFoundation/zebra/bb98b559634db822a9a10041e03acb3580688505/zebra-rpc/qa/rpc-tests/test_framework/"
zcash_base="https://raw.githubusercontent.com/zcash/zcash/master/qa/rpc-tests/test_framework/"

# Loop through the files array
for file in "${files[@]}"; do
    echo "Comparing $file"
    diff --color <(curl "${zebra_base}${file}" -s) <(curl "${zcash_base}${file}" -s)
done

That covers 19 of the files added in this PR.

Then, the proxy file, which is new, can be compared locally with the authproxy one as follows:

diff --color <(curl https://raw.githubusercontent.com/ZcashFoundation/zebra/bb98b559634db822a9a10041e03acb3580688505/zebra-rpc/qa/rpc-tests/test_framework/proxy.py -s) <(curl https://raw.githubusercontent.com/ZcashFoundation/zebra/bb98b559634db822a9a10041e03acb3580688505/zebra-rpc/qa/rpc-tests/test_framework/authproxy.py -s)

The only test moved can be compared with the one in zcashd as follows (no changes were done there, so the diff is empty):

diff --color <(curl https://raw.githubusercontent.com/ZcashFoundation/zebra/bb98b559634db822a9a10041e03acb3580688505/zebra-rpc/qa/rpc-tests/getmininginfo.py -s) <(curl https://raw.githubusercontent.com/zcash/zcash/master/qa/rpc-tests/getmininginfo.py -s)

@oxarbitrage oxarbitrage added C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces A-compatibility Area: Compatibility with other nodes or wallets, or standard rules P-Medium ⚡ and removed C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Sep 18, 2024
@oxarbitrage oxarbitrage self-assigned this Sep 18, 2024
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Sep 18, 2024
@oxarbitrage
Copy link
Contributor Author

Output:

alfredo@spaceship:~/zebra/zcash-rpc-tests/zebra/zebra-rpc$ ./qa/pull-tester/rpc-tests.py
..
getmininginfo.py:
Pass: True, Duration: 1 s
............
reindex.py:
Pass: True, Duration: 8 s
TEST             | PASSED | DURATION

getmininginfo.py | True   | 1 s
reindex.py       | True   | 8 s
ALL              | True   | 9 s (accumulated)
Runtime: 8 s
alfredo@spaceship:~/zebra/zcash-rpc-tests/zebra/zebra-rpc$ 

The above is what the CI needs to execute in the context of #8781

Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

Thank you!

@arya2 arya2 marked this pull request as ready for review September 20, 2024 16:13
@arya2 arya2 requested a review from a team as a code owner September 20, 2024 16:13
@arya2 arya2 requested review from upbqdn and removed request for a team September 20, 2024 16:13
mergify bot added a commit that referenced this pull request Sep 20, 2024
@mergify mergify bot merged commit c8280d4 into main Sep 20, 2024
97 checks passed
@mergify mergify bot deleted the zcash-rpc-tests-framework branch September 20, 2024 16:36
dmidem pushed a commit to QED-it/zebra that referenced this pull request Oct 29, 2024
…n#8866)

* move the rpc-tests framework from zcashd

* ignore pycache

* remove all tests from the list except getmininginfo

* iimprove a bit the readme

* change some env variable names

* add cache, add reindex test

* fix the paralell framework

* fix env variables

* change tests order

* update docs with env variable name change

* fix binary location

* reduce base config

* restore env var

* ignore stderr in the output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compatibility Area: Compatibility with other nodes or wallets, or standard rules A-rpc Area: Remote Procedure Call interfaces C-feature Category: New features C-testing Category: These are tests C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants