test: add comprehensive test coverage for AccessControl contracts #132
Conversation
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
Coverage Report
Last updated: Mon, 27 Oct 2025 03:56:15 GMT for commit |
adamgall
force-pushed
the
access-control-tests
branch
2 times, most recently
from
3b71b42 to
31323f0
Compare
maxnorm
reviewed
Oct 26, 2025
| uint256 gasUsed = gasBefore - gasleft(); | ||
|
|
||
| console2.log("Gas used for hasRole:", gasUsed); | ||
| assertEq(gasUsed, 1377, "hasRole gas cost should be exactly 1377"); |
Collaborator
There was a problem hiding this comment.
Using fix gas assumption, in test, break the coverage.
This gas value is based on the assumption of optimizer runs which the coverage action disable. The optimizer_runs value is set to 20_000, but this value can also be change. This will also break the tests.
Upper limit seems a good way to test gas usage within a appropriate range while i agree that it's not the most accurate way.
Collaborator
Author
There was a problem hiding this comment.
best way IMO is to rip out these "gas tests" entirely, and utilize forge snapshot for tracking gas costs between commits / code.
Collaborator
Author
|
will fix these broken actions |
adamgall
force-pushed
the
access-control-tests
branch
2 times, most recently
from
55ee4c5 to
90106c4
Compare
…rfect-Abstractions#131) Implement complete test suites for LibAccessControl and AccessControlFacet with test harnesses to expose internal library functions for thorough testing. New files added: - test/access/AccessControl/LibAccessControl.t.sol (454 lines, 37 tests) - test/access/AccessControl/AccessControlFacet.t.sol (611 lines, 45 tests) - test/access/AccessControl/harnesses/LibAccessControlHarness.sol - test/access/AccessControl/harnesses/AccessControlFacetHarness.sol Test coverage includes: - Core RBAC functionality (grant, revoke, renounce, hasRole) - Role admin hierarchies and custom role admins - Access control enforcement with requireRole - Storage slot verification and collision prevention - Event emission for all role operations - Edge cases (circular hierarchies, self-admin roles, zero addresses) - Comprehensive fuzz tests for all operations - Complex multi-level role hierarchy scenarios Total: 82 new tests ensuring robust access control implementation following patterns established by Owner and OwnerTwoSteps test suites. Closes Perfect-Abstractions#131
adamgall
force-pushed
the
access-control-tests
branch
from
90106c4 to
61d0e0d
Compare
JackieXu
pushed a commit
to JackieXu/Compose
that referenced
this pull request
Nov 6, 2025
…rol-tests test: add comprehensive test coverage for AccessControl contracts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
test: add comprehensive test coverage for AccessControl contracts (#131)
Implement complete test suites for LibAccessControl and AccessControlFacet
with test harnesses to expose internal library functions for thorough testing.
New files added:
Test coverage includes:
Total: 82 new tests ensuring robust access control implementation
following patterns established by Owner and OwnerTwoSteps test suites.
Closes #131