-
Notifications
You must be signed in to change notification settings - Fork 60
Implement BTT testing for LibERC20 #250
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
Implement BTT testing for LibERC20 #250
Conversation
👷 Deploy request for compose-diamonds pending review.Visit the deploys page to approve it
|
Coverage Report
Last updated: Wed, 17 Dec 2025 09:19:39 GMT for commit |
Gas ReportNo gas usage changes detected between All functions maintain the same gas costs. ✅ Last updated: Wed, 17 Dec 2025 09:20:22 GMT for commit |
adamgall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a really great approach to testing, and I'd like to see more of this for sure!
How do the *.tree files "work"? Are they consumed in any tooling?
Regarding your continued approach, I think option 2 is best to do the sooner it can happen. Maybe a hybrid approach would be to just build the new structure alongside what exists already, then when it's finished and at parity (or whatever), we can delete the old test structure.
If that sounds like too much, or you'd prefer something else, let me know.
I've been using these VSCode extensions:
I also feel the same way - option 2 is better in the long term. As far the workload, I'm more than happy to take it on. I'll chip away at it over the next month. Would you prefer this done in smaller, targeted PRs? That is, fuzzed unit testing for a single facet or module. |
|
@lumoswiz Looks good! Good job. Got an error on comment formatting: https://github.com/Perfect-Abstractions/Compose/actions/runs/20244602006/job/58121261728?pr=250 Can you fix the error on comment formatting? Then I can merge this. |
|
Okay, this is ready now. As I continue the test suite refactor, I will:
I'll do one PR per facet/module, unless grouping some (e.g. facet/module pairs) makes sense. |
|
@lumoswiz Great! |
|
@lumoswiz We are also going to need some documentation written in https://compose.diamonds/docs/contribution/testing that explains how the new testing works. I'm really glad you are working on this. |
Summary
LibERC20contract (i.e.ERC20Mod). See Adopt BTT and shared base contract across test suite #248.If this looks okay, I can continue with testing other smart contracts. In this case, I will need feedback/guidance on how to structure the test directory. I see two options in the short term:
ERC20Facetnext, I would add afacetfolder here:test/token/ERC20/ERC20/facet. Then add tree and test files for each function.Changes Made
LibERC20using the BTT style, with accompanying.treefiles.Checklist
Before submitting this PR, please ensure:
Code follows the Solidity feature ban - No inheritance, constructors, modifiers, public/private variables, external library functions,
using fordirectives, orselfdestructCode follows Design Principles - Readable, uses diamond storage, favors composition over inheritance
Code matches the codebase style - Consistent formatting, documentation, and patterns (e.g. ERC20Facet.sol)
Code is formatted with
forge fmtExisting tests pass - Run tests to be sure existing tests pass.
New tests are optional - If you don't provide tests for new functionality or changes then please create a new issue so this can be assigned to someone.
All tests pass - Run
forge testand ensure everything worksDocumentation updated - If applicable, update relevant documentation