-
Notifications
You must be signed in to change notification settings - Fork 150
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
🧹 Added unit tests for MyONFT721.sol example #964
base: main
Are you sure you want to change the base?
Conversation
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.
Looks pretty good overall, although the new tests currently aren't running. Please let me know if you have any questions!
@@ -0,0 +1,152 @@ | |||
// SPDX-License-Identifier: UNLICENSED |
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.
These tests are not run. You need to modify the "test" script in package.json to:
"test": "$npm_execpath run test:forge && $npm_execpath run test:hardhat",
// DevTools imports | ||
import { TestHelperOz5 } from "@layerzerolabs/test-devtools-evm-foundry/contracts/TestHelperOz5.sol"; | ||
|
||
contract MyONFT721Test is TestHelperOz5 { |
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.
Are these new tests? Why wouldn't you just import the ones from:
ONFT721Mock private aONFT721; | ||
ONFT721Mock private bONFT721; | ||
|
||
address private userA = address(0x1); |
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.
Use makeAddr("userA")
and makeAddr("userB")
instead. This will enhance the error logs by replacing the address with the canonical name.
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.
Tests need to be running, see comments for further information on how to enable them.
Added test script line to package.json |
No description provided.