infra: Foundry CI workflow + foundry.toml (closes #32)#34
Merged
Conversation
Muzix ships three Forge suites (MUSD.t.sol, MuzixCatalog.t.sol, MuzixAIProvenance.t.sol from #29) but has no Foundry CI — they only get exercised when contributors run `forge test` locally. That's the regression window this PR closes. Changes: - `foundry.toml` — standard profile: solc 0.8.24 (matches `pragma` in src/*), optimizer on with 200 runs, remappings for `@openzeppelin/` (installed from npm) and `forge-std/` (installed as a submodule by the workflow). - `.github/workflows/forge.yml`: - Trigger: push / PR to main or master. - Setup: `foundry-rs/foundry-toolchain@v1` pinned to `stable`. - Bootstraps `lib/forge-std` on first run; uses `npm install @openzeppelin/contracts@^5.0.0` to satisfy the OZ remapping. - Runs `forge build --sizes`, `forge test -vvv`, then `forge fmt --check` (non-blocking for now — flip to blocking in a follow-up once a fmt sweep lands). - `.gitignore` — ignore `node_modules/`, `out/`, `cache_forge/`, `lib/forge-std` so CI-bootstrapped deps don't trip up contributors. Closes #32. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
abhicris
force-pushed
the
infra/foundry-ci
branch
from
aaaa5cf to
7bc7ecb
Compare
The CI workflow in this PR is now actually running `forge test` — that exposed three pre-existing failures on master. This fixes all three so the PR merges green. 1. MuzixCatalog.t.sol — `testMintAndMetadata` / `testRoyaltySplitSetsPoolRoyalty` both reverted with `ERC721InvalidReceiver`. `mintMusic()` calls `_safeMint(msg.sender, ...)` and `msg.sender` is the test contract itself (the deployer / owner). The test contract didn't implement `IERC721Receiver`. Added the `onERC721Received` hook returning its selector so the contract can hold tokens. 2. MuzixAIProvenance.t.sol::testRevertWhenTooManyModels — expected `TooManyModels(17, 16)`, actual `NotTokenOwner(..., artist_addr)`. Root cause: `registry.MAX_AI_MODEL_TOKENS()` inside the `abi.encodeWithSelector(...)` argument to `vm.expectRevert` is an external call to `registry` — it consumed the `vm.prank(artist)` before the real `setProvenance` call. Moved the external read out of the expectRevert expression to before the prank; the prank now actually applies to `setProvenance` and the test reaches the `TooManyModels` check. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #32. Muzix ships three Forge suites (MUSD.t.sol, MuzixCatalog.t.sol, MuzixAIProvenance.t.sol from #29) but has no Foundry CI — they only get exercised when contributors run forge locally.
Changes
foundry.toml— standard profile: solc 0.8.24 (matchespragmainsrc/*), optimizer on (200 runs), remappings for@openzeppelin/(installed from npm) andforge-std/(submodule, bootstrapped by the workflow)..github/workflows/forge.yml— trigger on push / PR to main or master.foundry-rs/foundry-toolchain@v1 stable, bootstrapslib/forge-std,npm install @openzeppelin/contracts@^5.0.0to satisfy the OZ remapping, runsforge build --sizes+forge test -vvv+forge fmt --check(non-blocking for now — flip to blocking in a follow-up once a fmt sweep lands)..gitignore— ignorenode_modules/,out/,cache_forge/,lib/forge-std.Verification
forge build --sizes+forge test -vvvto exercise the three.t.solfiles shipped so far.Follow-ups
forge fmtbaseline lands, flipforge fmt --checkto blocking.forge coveragejob with artifact upload.Test plan
forge-std— kcolbchain / Abhishek Krishna