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

fix(ci): allow to run all lighwalletd tests in Docker #9038

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

gustavovalverde
Copy link
Member

Motivation

A LWD test was expecting the ZEBRA_TEST_LIGHTWALLETD to be set, but this variable is needed for all LWD tests and not specifically for lightwalletd_integration.

We had to rename this variable on a buggy elif statement in our Docker entrypoint.

This was avoiding most LWD tests to run correctly.

Solution

  • Use $TEST_LWD_INTEGRATION to run lightwalletd_integration in our entrypoint and in CI
  • Remove $ZEBRA_TEST_LIGHTWALLETD as this was an incorrect logic

Tests

  • We must validate this tests are running correctly, by checking the logs directly in this PR
  • 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.

A LWD test was expecting the `ZEBRA_TEST_LIGHTWALLETD` to be set, but this variable is needed for all LWD tests and not specifically for `lightwalletd_integration`.

We had to rename this variable on a buggy `elif` statement in our Docker entrypoint.

This was avoiding most LWD tests to run correctly.
@gustavovalverde gustavovalverde added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles I-integration-fail Continuous integration fails, including build and test failures P-Critical 🚑 labels Nov 18, 2024
@gustavovalverde gustavovalverde self-assigned this Nov 18, 2024
@gustavovalverde gustavovalverde requested a review from a team as a code owner November 18, 2024 14:55
@gustavovalverde gustavovalverde requested review from arya2 and removed request for a team November 18, 2024 14:55
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Nov 18, 2024
@gustavovalverde gustavovalverde changed the title fix(ci): run most lighwalletd tests correctly fix(ci): allow to run all lighwalletd tests in Docker Nov 18, 2024
@gustavovalverde
Copy link
Member Author

@mergify queue

Copy link
Contributor

mergify bot commented Nov 19, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 3983428

@gustavovalverde
Copy link
Member Author

The lightwalletd_wallet_grpc_tests are failing in this PR, but that's a different issue. Let's merge this one so we can continue the Rust fix in another one https://github.com/ZcashFoundation/zebra/actions/runs/11895020195/job/33189274878#step:15:721

mergify bot added a commit that referenced this pull request Nov 19, 2024
@mergify mergify bot merged commit 3983428 into main Nov 19, 2024
181 of 183 checks passed
@mergify mergify bot deleted the fix-lwd-tests branch November 19, 2024 11:42
@arya2 arya2 mentioned this pull request Dec 5, 2024
45 tasks
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 C-bug Category: This is a bug C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG I-integration-fail Continuous integration fails, including build and test failures P-Critical 🚑
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants