Improve GitHub Actions for faster checks and better readability

[gustavovalverde]

Motivation

All GitHub Actions workflows that are triggered by an event, are being run independently of the files or folder changed. Making all PRs take the same amount of CI time and going through the whole test suite.

Depending on the files being changed, specific workflows must run, not all of them if not needed.

Related to: #3359
Depends-On: #3386

Solution

• Move linting and styling to a specific workflow, making each workflow have a specific responsibility
• Add paths to workflows triggered by a GitHub event
• CI: Do not run twice (on push to main and PRs to main), just run on PRs as Mergify also runs the CI before merging with the latest tip from main. Just run on changes to Rust files, Cargo files, or the Dockerfile.
• Coverage: Same as CI; excluding the Dockerfile.
• Docs: Just when the book/ folder is changed or any firebase configuration.
• Lint: Just run on changes to Rust files or the Clippy configuration.
• Test: Same as CI; adding the book/ folder.

Review

The whole @ZcashFoundation/zebra-team should review this.

Follow Up Work

Add Check deny.toml crate dependencies and validate licenses to the list of required checks for each PR, in the Zebra GitHub repository settings.

Another PR will include better use of GitHub variables, removing not-needed steps, using major versions on actions, and allowing automatic bug fixes. It will also improve overall readability and secret management.

Edit: Moving pull-request (zealous-zebra) to a GitHub Action.

[codecov]

Codecov Report

Merging #3377 (ed4f4ce) into main (499ae89) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3377      +/-   ##
==========================================
- Coverage   78.34%   78.26%   -0.08%     
==========================================
  Files         267      267              
  Lines       31526    31526              
==========================================
- Hits        24698    24675      -23     
- Misses       6828     6851      +23     

[mpguerra]

I wonder which tests we want to run when tagging a new release, e.g PR #3256

These PRs typically change:

• CHANGELOG.md
• README.md
• zebra-network/src/constants.rs < this is probably the riskiest change...
• Cargo.lock, and
• all of the Cargo.toml files for the crates for which we are incrementing the release number.

On the one hand it would be nice to run the whole test suite, on the other hand it doesn't really seem necessary and can delay tagging the release sometimes...

@ZcashFoundation/zebra-team any thoughts?

[teor2345]

I don't think there's any way to avoid running the full test suite for changes to these files:

• zebra-network/src/constants.rs
• Cargo.lock

Since cargo lock can change dependency versions, we need to run the full test suite on any changes to these files. We could automate updating the user agent in the zebra network constants, but that wouldn't actually change the overall risk of tagging a release.

Instead, if our tests are running too slowly, we should consider paying for a bigger machine to run tests on.

[teor2345]

This seems like a useful change, but the path inclusions could be quite risky.

How do we know that we haven't accidentally missed some paths that should be checked?
How will we make sure we keep the list of paths up to date as we make changes to Zebra?

Can we exclude paths that we are sure aren't needed, rather than trying to track all of the paths that are needed?
Using an exclude list is less risky, because if we forget to exclude some files, then CI just becomes a bit slower. But if we forget to include some files, we can merge bugs into the zebra main branch.

I also wonder about the GitHub CI cache limit. We are already hitting that limit, and adding more jobs will increase our cache usage. So we should check that this new set of jobs is actually faster, particularly after the caches have been populated.

[gustavovalverde]

How do we know that we haven't accidentally missed some paths that should be checked?
How will we make sure we keep the list of paths up to date as we make changes to Zebra?

At least for the most critical workflows (CI, Coverage, Lint, and Test), we're covering over 99% of the codebase with:

• **/*.rs

And considering most critical changes with:

• **/*.txt
• **/Cargo.toml
• **/Cargo.lock

[teor2345]

How do we know that we haven't accidentally missed some paths that should be checked?
How will we make sure we keep the list of paths up to date as we make changes to Zebra?

At least for the most critical workflows (CI, Coverage, Lint, and Test), we're covering over 99% of the codebase

Ok, that seems fine for now. If some bugs get through CI, we can fix it then.

[teor2345]

Looks good, thanks for splitting out the functionality changes into a separate PR.

I just have a few questions about when jobs need to run.

Major issues got fixed

[teor2345]

@gustavovalverde this PR splits a CI job that is required to merge. So we need to add the new Check deny.toml crate dependencies and validate licenses job to the Zebra GitHub repository config.

(The dependency checks are important, so they should be required to merge.)

[teor2345]

This looks good to me, but maybe we should get @dconnolly to do a review as well.