Testing WG: Proposal Thread

[AshtonStephens]

Proposal for CI Unification [August 7th, 2023]

Background

This is a document that contains TrustMachine's CI principles. Going into this discussion, it has been reviewed by every engineering member of Trust Machines in addition to @wileyj.

I'm opening this discussion so that these tenets might be adopted by the wider community. At the moment we plan to implement these principles into the stacks-network/sbtc and judge its effectiveness.

---

Trust Machines CI Design Philosophy

1. Introduction

1.1 Document Purpose

This document will provide a framework for making design decisions around our development procedures and CI / CD pipelines. It will help us weigh the pros and cons of different design options, and make decisions that are aligned with the overall philosophy for future decision making. This document will prevent us from repeating philosophical conversations once a consensus has been met.

1.2 Glossary

Term	Description
Continuous integration	A practice where developers regularly merge their code changes into a shared repository, where automated builds and tests are run.
Local Testing	Software testing that is performed on a local machine of a developer.
Cloud Testing	Software testing performed on a remote server that relies on existing cloud infrastructure.
Pull Request	A change to code that needs to be reviewed before merging into a branch with production or development impact
Automatic Validation	Tests and code quality scripts that run on a pull request that automatically verify the quality of a pull request into either the acceptable or unacceptable category

2. Purpose of Automatic Validation and CI

The purpose of a CI/CD pipeline is to:
Cut down on review time with automatic pre-review validation
Make released features more reliable.
Reduce the manual time spent on manual verification

3. Challenges

3.1 Open Source Collaboration

Our software will eventually be written and maintained by a mix of open source volunteers and members of the Trust Machines organization. This means that we cannot depend on reliable resources and knowledge of the open source community.

3.2 Fast Feature Rollout

Our software requires tight turnaround times to keep pace with the bitcoin ecosystem. This makes it difficult to have a robust one-size-fits-all continuous integration system that does not hinder the ability to push out code quickly.

4. CI Design Tenets

4.1 Passing Automatic Validation is Never a Suggestion

If PRs routinely fail the automatic validation but are allowed to merge then the automatic validation steps are incorrect and over cautious. Continuous integration pipeline steps must never be a suggestion. If a check fails then the pr is not up to snuff and must be rejected or rewritten. This means that unless a pr is a hotfix, a red x next to a pull request makes it unworthy of review.

4.2 PR Validation Should Only Fail on New Changes

If PRs fail the automatic validation, the cause of the failure should be introduced in the PR. Continuous integration pipeline steps blocking PRs from being merged may only do so if they have just cause. This means that if a PR which fails automatic validation reverts all of its changes, the resulting empty PR should pass automatic validation.

4.3 The PR Validation Must Be Easy to Run Locally

All individual features must be testable with unit tests alone and require little cloud infrastructure. By requiring cloud testing to pass initial automatic validation we introduce a significant barrier of entry to outside collaboration and are trusting that the cloud infrastructure is set up to test the new code in an iterative manner. The iteration time of bug fixes sky rockets if cloud infrastructure is required for feature tests. If automatic testing cannot be reproduced locally then pull requests will be opened with incomplete solutions.

4.4 Continuous Integration Pipeline must Only Check Code Relevant to Production

If it does not have bearing on the behavior on production then testing resources should not be used to verify it. Code relevant to debugging should be tested locally if it needs testing and be intended for use in active investigation. Default monitoring falls under production behavior.

• YES: verify metrics are produced on an error
• NO: test with cargo run build instead of or in addition to cargo run build --release

4.5 Resource Heavy Tests Should Run Only on Approved Code

No resource heavy tests should be required for a pull request to be approved by automatic validation. Resource intensive tests are the hardest to iterate on and the most financially expensive to repeat. If we put resource heavy tests into pull request validation then we will start to over rely on them for catching mistakes which will dramatically increase iteration time and again increase the barrier of entry outside collaboration.

4.6 Resource Heavy Cloud Testing Infrastructure Does Not Need to be Reproducible by the Outside Community, but should be accessible to them.

The outside community must be able to contribute, but does not need to have the same level of resources as we do. By expecting the open source community to reproduce high infrastructure tests we make it more difficult to contribute without having the resources or knowledge to maintain or create the infrastructure themselves. Ideally our infrastructure will be open for them to observe but not necessarily create.

5. Conclusion

These tenets will help us to create a CI/CD pipeline that is both effective and efficient, while also being accessible to the open source community. By straying from some of these tenets we reduce the effectiveness of our infrastructure and make it less reliable in cutting down development time.

[wileyj]

One thing that's always interested me as well is the ability to scan open CVE's and compare to any crates we've added as dependencies.
if a cve is found, the build should fail until the dependency is upgraded to a patched version, or versioned in the code.

there sort of exists an action for this already, but it's not maintained - might be worth the effort to fork and maintain a lot of these rust actions from the actions-rs github org, some are pretty useful but none are actively maintained.

https://github.com/actions-rs

[AshtonStephens]

We can always add cargo-audit and install it and then run. I think when it comes to github actions that are wrappers over a single rust command (excluding the install) it would be better to keep it simple so it can more obviously be recreated on the command line. If it's more complicated than a single line then github actions all the way.

What do you think?

[AshtonStephens]

Though, I see how cargo-audit goes against the principles. Maybe there's more to be added to this discussion around dependency management.

[wileyj]

i can see the appeal of kiss, but with an workflow it's quite nice to have something that runs the command under the hood and then outputs something to the action UI:
"audit failed. here's why: xxxx"

an example (incidentally, a forked verions of https://github.com/actions-rs/audit-check): stacks-network/clarity-wasm#25.

this is really just cargo audit behind the scenes, but it has the ability to create issues which is nice. it's a bit more nuanced than that though, because some of these workflows can be chatty (as i've noticed with audit-check and clippy in particular).

in general though, i'd support using workflows like these - but we'd want to find what works for us (and possibly forking the repos to do things specifically if we require them).

[wileyj]

what i've found with simply running the command via run: cargo blah is that it'll run as you think it will - but digging into failures in the workflow logs is not a pleasant experience.