test: ACL unit tests with foundry #235
Conversation
PacificYield
force-pushed
the
acl-foundry
branch
from
03bcc28 to
4389f62
Compare
contracts/hardhat.config.ts
Outdated
| @@ -1,3 +1,4 @@ | |||
| import '@nomicfoundation/hardhat-foundry'; | |||
There was a problem hiding this comment.
PacificYield
force-pushed
the
acl-foundry
branch
2 times, most recently
from
7e35d1d to
8a0aa3e
Compare
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| - run: cp ./contracts/.env.example ./contracts/.env | ||
| - run: npm --prefix ./contracts ci --include=optional | ||
| - run: npm --prefix ./contracts run prettier:check | ||
| - name: "npm CI test" | ||
| - name: Prettier check |
There was a problem hiding this comment.
I think the prettier check should be its own workflow
There was a problem hiding this comment.
Can be done in another PR, it is similar in other repos (prettier checks being part of the test workflow)
contracts/test/acl/acl.t.sol
Outdated
| import {tfheExecutorAdd} from "../../addresses/TFHEExecutorAddress.sol"; | ||
|
|
||
| /// @dev It is used for checking that _authorizeUpgrade() is gatekept. | ||
| contract ACLWithUpgrade is ACL { |
There was a problem hiding this comment.
why is this new contract needed?
There was a problem hiding this comment.
Remove it and replace it by a call to upgradeToAndCall from the inherited UUPS.
|
Please remove the ACLWithUpgrade contract in your test, you don't need it and it makes the code less readable. |
PacificYield
force-pushed
the
acl-foundry
branch
from
8a0aa3e to
35a991e
Compare
PacificYield
changed the title
PacificYield
force-pushed
the
acl-foundry
branch
from
35a991e to
9a63076
Compare
PacificYield
changed the title
|
I just tested the deployment script of this branch and I noticed it breaks on linux VM without foundry installed with "Error in plugin hardhat-foundry". This is really bad. Please don't use this hardhat plugin anymore and put the foundry tests elsewhere in a separate folder inside contracts/. We really don't want to clutter deployment scripts with unnecessary dependencies to make ZWS team job easier. |
contracts/test/acl/acl.t.sol
Outdated
| acl.allowForDecryption(handlesList); | ||
| } | ||
|
|
||
| function testFail_OnlyOwnerCanAuthorizeUpgrade(address randomAccount) public { |
There was a problem hiding this comment.
it would be better here to test the specific error of wrong owner.
forge coverage --report lcov && genhtml lcov.info -o report --branch-coverage