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

matrix foundry tests and snapshots #10022

Merged
merged 18 commits into from
Aug 15, 2023
Merged

Conversation

RensR
Copy link
Contributor

@RensR RensR commented Aug 1, 2023

This PR aims to improve the developer experience for developers writing Solidity through the following changes

  • Matrix testing for Foundry tests
    • Each product (llo-feeds, vrf, ...) runs their Foundry tests in parallel
    • Clear signalling on which product would break CI
    • Currently it still runs all profiles regardless if a given product folder has changes. This is partly due to the Solidity folder not being properly cleaned up yet and many products are not contained to their designated folder.
    • Also includes matrix based gas snapshot checks (see Per product gas snapshots section below)
  • Moved the Makefile for all Solidity based logic to /contracts so developers can run Foundry tests and the Make commands from the same location.
  • Per product gas snapshots
    • Each product has its own gas snapshot, meaning updating the snapshot doesn't have to compile every contract
    • Due to Foundry's caching limitation on compiling across Solidity versions, updating the gas snapshot always took significantly longer than just running tests for any given product.
    • Based on the FOUNDRY_PROFILE env var and a new Make command make snapshot
    • Since devs usually have the FOUNDRY_PROFILE already set during development to run forge test they can now run make snapshot to update only the snapshot for the product they're currently working on
  • Add product specific wrapper generation Make commands
    • Uses the same FOUNDRY_PROFILE to indicate which wrappers are generated
    • Also works for non-Foundry products
    • Example code make wrappers with env var FOUNDRY_PROFILE=llo would compile all llo contracts and then create gethwrappers just for llo
    • Is based on the split native_solc_compile_all_XXX compile scripts and core/gethwrappers/$(FOUNDRY_PROFILE)/go_generate.go generation files.
      • Not all products have a product specific go_generate yet, if you want to use this functionality, simply extract the lines that apply to your product into a properly named file to add support
  • Split generic Solidity CI and Hardhat CI
    • Add ignore pattern to Hardhat CI to ignore product folders that don't use Hardhat
    • This means that Hardhat CI can be skipped if changes are contained to those folders, speeding up CI very significantly as the new split Foundry CI takes only a single minute
      • You'll never have to wait for a ubuntu20.04-4cores-16GB machine to be available to run HH jobs

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2023

I see that you haven't updated any README files. Would it make sense to do so?

@RensR RensR force-pushed the matrix-foundry-tests-&-snapshots branch from 1d9a6c3 to 6722e11 Compare August 3, 2023 15:00
@RensR RensR force-pushed the matrix-foundry-tests-&-snapshots branch from 49dfd88 to aa7f705 Compare August 3, 2023 15:13
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

@RensR RensR force-pushed the matrix-foundry-tests-&-snapshots branch from 0520558 to d6055da Compare August 3, 2023 15:35
@RensR RensR force-pushed the matrix-foundry-tests-&-snapshots branch from d6055da to 669638e Compare August 3, 2023 15:36
@RensR RensR marked this pull request as ready for review August 4, 2023 10:00
# ALL_FOUNDRY_PRODUCTS contains a list of all products that have a foundry
# profile defined. Adding a product to this list will make it available for
# snapshotting.
ALL_FOUNDRY_PRODUCTS = vrf automation llo functions automation-dev shared
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

automation-dev is required because automation has Foundry code in the dev folder. This should be moved into the automation folder at some point.

shared is already here even though there are no Foundry tests yet, this will soon change so I already added it

id: changes
with:
filters: |
src:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This excludes quite a few files vs the old filter. Hopefully this results in running HH less often while never missing a run when needed

@@ -2,5 +2,5 @@
pragma solidity ^0.8.0;

interface ITypeAndVersion {
function typeAndVersion() external pure virtual returns (string memory);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing small mistake from previous PR

Copy link
Collaborator

@chainchad chainchad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI looks good, just a few small suggestions.

.github/workflows/solidity-foundry.yml Show resolved Hide resolved
.github/workflows/solidity-hardhat.yml Show resolved Hide resolved
.github/workflows/solidity-hardhat.yml Show resolved Hide resolved
.github/workflows/solidity-hardhat.yml Show resolved Hide resolved
contracts/GNUmakefile Outdated Show resolved Hide resolved
contracts/GNUmakefile Outdated Show resolved Hide resolved
.PHONY: wrappers
wrappers: pnpmdep abigen ## Recompiles solidity contracts and their go wrappers.
./scripts/native_solc_compile_all_$(FOUNDRY_PROFILE)
go generate ../core/gethwrappers/$(FOUNDRY_PROFILE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense, but existing wrappers are placed under /core/gethwrappers/generated folder today, could this cause issues for go imports?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works for projects that adopt the new location. If they use the old way, they have to run make wrappers-all, just like now, and their files are unchanged. Some projects have already moved their wrapper generation code to subfolders, this command if for those teams.

@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@RensR RensR added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2023
@RensR RensR added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Aug 14, 2023
@RensR RensR added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2023
@matYang matYang added this pull request to the merge queue Aug 15, 2023
Merged via the queue into develop with commit 67b1f2d Aug 15, 2023
103 checks passed
@matYang matYang deleted the matrix-foundry-tests-&-snapshots branch August 15, 2023 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants