privacy-scaling-explorations / zk-kit

A monorepo of reusable libraries for zero-knowledge technologies.
https://zkkit.pse.dev/
MIT License
257 stars 53 forks source link

ci: granular workflow #266

Closed baumstern closed 3 weeks ago

baumstern commented 3 weeks ago

After discussing with @cedoor, we identified that the current CI workflow could be improved by taking a more granular approach. Specifically, since this repo is a monorepo, the CI triggers whenever a PR is opened, and it builds and tests every package. We could instead trigger only the workflows related to the changed packages and build and test just those packages. By reducing the overhead required to run the overall build and test steps, we could improve the CI lead time by several seconds or even minutes, which would provide much faster feedback to the contributor.

cedoor commented 3 weeks ago

cc @sripwoud

sripwoud commented 3 weeks ago

I think this can be achieved by configuring properly the turbo cache in the ci. Something I postponed when doing #235. This is what I am doing in the anonklub repo https://github.com/anonklub/anonklub/blob/3813ee207327aa26f6e1bb2a4297b26e770a5f56/.github/workflows/static-analysis.yml#L52-L58

Or do you know another way to achieve it @baumstern? Another idea might to split everything in different files/workflows with packages paths filter in each. But I don't like this solution because it will be hard to reflect package inter dependencies (while turbo understands the monrepo topology) and it means lot of duplicated code in the workflow files.

baumstern commented 3 weeks ago

I think this can be achieved by configuring properly the turbo cache in the ci. Something I postponed when doing #235. This is what I am doing in the anonklub repo https://github.com/anonklub/anonklub/blob/3813ee207327aa26f6e1bb2a4297b26e770a5f56/.github/workflows/static-analysis.yml#L52-L58

Or do you know another way to achieve it @baumstern? Another idea might to split everything in different files/workflows with packages paths filter in each. But I don't like this solution because it will be hard to reflect package inter dependencies (while turbo understands the monrepo topology) and it means lot of duplicated code in the workflow files.

It seems like that using cache does save times of preparing build dependencies, but running tests, especially tests against contracts incurs approx 13 minutes. Also there is another CI overhead of transpiling ts into js that incurs approx. 35 seconds.

When a PR introduces changes to specific components like circuits, selectively skipping tests for unrelated contracts (e.g., not running tests for lazytower.sol and imt.sol if they are unaffected) could reduce the CI process time to about 3-5 minutes, down from roughly 13 minutes.

We could use path filters, or alternatively, write custom scripts to bypass irrelevant tests. Here’s a conceptual example of how we might conditionally execute tests: e.g.

- name: Test ${{ matrix.type }}
              if: github.event_path == 'packages/*.sol' && matrix.type == 'contracts'
              run: yarn test:${{ matrix.type }}

(note that github.event_path == 'packages/*.sol' condition will not work actually, custom script should be used intead)