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

Create an integration test where we make requests to lightwalletd backed by zebrad #3510

Closed
3 tasks done
Tracked by #3134
dconnolly opened this issue Feb 10, 2022 · 7 comments
Closed
3 tasks done
Tracked by #3134
Assignees
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-rust Area: Updates to Rust code C-testing Category: These are tests lightwalletd any work associated with lightwalletd

Comments

@dconnolly
Copy link
Contributor

dconnolly commented Feb 10, 2022

Motivation

We want to check that Zebra actually works with lightwalletd.

Tasks

Stage 1

The test should pass if lightwalletd compiles.

Stage 2

  • Add a lightwalletd integration test for the RPC port, here is an example
  • Run lightwalletd_integration acceptance test in CI after setting ZEBRA_TEST_LIGHTWALLETD

Test Design

The test should pass if we get a "missing RPC" error for the next RPC method that lightwalletd calls after getblockchaininfo.

(Zebra already has a getblockchaininfo stub. It is ok if getblockchaininfo is missing some fields, Go will fill in zero-valued data.)

Follow-Up Tasks

@dconnolly dconnolly added S-needs-triage Status: A bug report needs triage C-testing Category: These are tests A-rust Area: Updates to Rust code P-Medium ⚡ labels Feb 10, 2022
@dconnolly
Copy link
Contributor Author

@teor2345

This comment was marked as resolved.

@teor2345
Copy link
Contributor

I added a link to a manual test to the ticket description:
#3589 (comment)

@teor2345
Copy link
Contributor

We will need to set ZEBRA_TEST_LIGHTWALLETD to get this test to run, after PR #3627 merges.

@teor2345
Copy link
Contributor

@gustavovalverde I just updated the ticket to use the ZecWallet fork's master branch. Can you make that change in CI?

@gustavovalverde
Copy link
Member

@teor2345 Sure.

@gustavovalverde
Copy link
Member

gustavovalverde commented Mar 7, 2022

Fixed by #3619 and #3657

@ftm1000 ftm1000 added the lightwalletd any work associated with lightwalletd label Mar 16, 2022
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-rust Area: Updates to Rust code C-testing Category: These are tests lightwalletd any work associated with lightwalletd
Projects
None yet
Development

No branches or pull requests

5 participants