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

add(scan): Add the scan_start_where_left test to CI #8172

Merged
merged 3 commits into from
Jan 23, 2024
Merged

Conversation

upbqdn
Copy link
Member

@upbqdn upbqdn commented Jan 19, 2024

Motivation

Close #8085.

PR Author Checklist

Check before marking the PR as ready for review:

  • Will the PR name make sense to users?
  • Does the PR have a priority label?
  • Have you added or updated tests?
  • Is the documentation up to date?

Solution

  • Add a new job named scan-start-where-left-test to the Integration Tests on GCP workflow.

Reviewer Checklist

Check before approving the PR:

  • Does the PR scope match the ticket?
  • Are there enough tests to make sure it works? Do the tests cover the PR motivation?
  • Are all the PR blockers dealt with?
    PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

@upbqdn upbqdn added A-devops Area: Pipelines, CI/CD and Dockerfiles C-testing Category: These are tests A-blockchain-scanner Area: Blockchain scanner of shielded transactions P-Medium ⚡ labels Jan 19, 2024
@upbqdn upbqdn self-assigned this Jan 19, 2024
@upbqdn upbqdn requested a review from a team as a code owner January 19, 2024 00:28
@upbqdn upbqdn requested review from oxarbitrage and removed request for a team January 19, 2024 00:28
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Jan 19, 2024
@upbqdn
Copy link
Member Author

upbqdn commented Jan 19, 2024

Let's see if the new job actually runs in the CI.

@upbqdn upbqdn force-pushed the add-scan-test-to-ci branch 2 times, most recently from a166a8c to 147ca1e Compare January 19, 2024 21:29
@upbqdn upbqdn force-pushed the add-scan-test-to-ci branch from 147ca1e to 0ba1e63 Compare January 19, 2024 22:06
arya2
arya2 previously approved these changes Jan 19, 2024
.github/workflows/ci-integration-tests-gcp.yml Outdated Show resolved Hide resolved
docker/entrypoint.sh Outdated Show resolved Hide resolved
@upbqdn
Copy link
Member Author

upbqdn commented Jan 20, 2024

@arya2 thanks for the fix!

I looked at our Dockerfile, and I noticed we build the test binaries when building the image: https://github.com/zcashfoundation/zebra/blob/88dd18adb7db6e6127da578d9d7aaa7cab869fe9/docker/Dockerfile#L133-L134.

The new job in this PR runs the tests here https://github.com/zcashfoundation/zebra/blob/88dd18adb7db6e6127da578d9d7aaa7cab869fe9/docker/entrypoint.sh#L183 with a different set of features. That means the tests are rebuilt each time the job runs anyway. I wonder if we could omit building the tests in the Dockerfile.

Edit: The answer is no because only a small subset of the crates is recompiled. If we omitted the building in the Dockerfile, each job would need to rebuild all of the crates.

@upbqdn upbqdn removed the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Jan 20, 2024
mergify bot added a commit that referenced this pull request Jan 23, 2024
@oxarbitrage oxarbitrage added the do-not-merge Tells Mergify not to merge this PR label Jan 23, 2024
mergify bot added a commit that referenced this pull request Jan 23, 2024
@oxarbitrage oxarbitrage removed the do-not-merge Tells Mergify not to merge this PR label Jan 23, 2024
mergify bot added a commit that referenced this pull request Jan 23, 2024
@mergify mergify bot merged commit 34a3ba8 into main Jan 23, 2024
151 checks passed
@mergify mergify bot deleted the add-scan-test-to-ci branch January 23, 2024 23:07
@upbqdn upbqdn mentioned this pull request Feb 23, 2024
43 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions A-devops Area: Pipelines, CI/CD and Dockerfiles C-testing Category: These are tests P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add scan_start_where_left test to CI
3 participants