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

devops: Align tests names in CI, Docker and Rust code #7541

Open
gustavovalverde opened this issue Sep 13, 2023 · 6 comments
Open

devops: Align tests names in CI, Docker and Rust code #7541

gustavovalverde opened this issue Sep 13, 2023 · 6 comments
Assignees
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-bug Category: This is a bug C-enhancement Category: This is an improvement I-usability Zebra is hard to understand or use S-needs-triage Status: A bug report needs triage

Comments

@gustavovalverde
Copy link
Member

gustavovalverde commented Sep 13, 2023

Describe the issue or request

It's hard for the team to know which CI test ID is running a specific Rust test, and how those matches with the Docker entrypoint.

Expected Behavior

Have a single name convention for tests across the project:

  • CI Job Names
    • changing these requires admin updates to branch protection rules
  • CI Test IDs
  • Rust Code
  • Docker entrypoint environment variables
    • some environment variables are used inside the Rust code itself, so they can't be changed

Current Behavior

We have a test called test-empty-sync in CI, but in Rust it actually triggers with the sync_large_checkpoints_{mainnet,testnet} test names.

Or having test-lightwalletd-integration in CI, which also requires a ZEBRA_TEST_LIGHTWALLETD ENV variable, to trigger the lightwalletd_integration Rust test.

Possible Solution

Use the same test names and try to trigger the test passing CLI parameters instead of ENV variables.

Do a mass rename to reliably change the names in scripts, workflows, Rust, and documentation:
https://github.com/ZcashFoundation/zebra/blob/main/book/src/dev/mass-renames.md

Avoid changing job names and job IDs, or do it in a separate PR, because updating branch protection rules is a manual task.

CI Test IDs should be limited to 20 characters, otherwise long branch names will fail (see bug #7559).

Additional Information/Context

No response

Is this happening on PRs?

N/A

Is this happening on the main branch?

N/A

@gustavovalverde gustavovalverde added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles S-needs-triage Status: A bug report needs triage C-enhancement Category: This is an improvement P-High 🔥 I-usability Zebra is hard to understand or use labels Sep 13, 2023
@gustavovalverde
Copy link
Member Author

Hey team! Please add your planning poker estimate with Zenhub @teor2345 @upbqdn @oxarbitrage @arya2

@teor2345
Copy link
Contributor

I was just thinking of this change today, when I had to copy almost exactly the same workflow arguments between a full sync and an update sync. If we used consistent names, it would be easier to see which jobs are similar to each other.

(And maybe in the future we could create those syncs from the same workflow, which contains all the detailed arguments. So the top-level workflow would just need a full/update argument. Or maybe that's too complicated.)

@teor2345
Copy link
Contributor

I added "CI Test IDs" to the list of names, with this note:

CI Test IDs should be limited to 20 characters, otherwise long branch names will fail (see bug #7559).

@mpguerra
Copy link
Contributor

@gustavovalverde have you also been working on this one?

@teor2345
Copy link
Contributor

I added some notes to the PR description about the names that are hard to change, because we need to change branch protection rules as well.

I think we could do this ticket relatively quickly by changing most environment variables to Rust test names in the entrypoint script. (Some environment variables are used inside the Rust code and can't be changed.)

Then we find everywhere those environment variables are used and rewrite those commands.

@gustavovalverde
Copy link
Member Author

@gustavovalverde have you also been working on this one?

@mpguerra Not yet

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-enhancement Category: This is an improvement I-usability Zebra is hard to understand or use S-needs-triage Status: A bug report needs triage
Projects
None yet
Development

No branches or pull requests

3 participants