-
Notifications
You must be signed in to change notification settings - Fork 619
[SDK] Consolidate more tests - reduce contract deployment #5258
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import { | |
| import { resolveContractAbi } from "../../contract/actions/resolve-abi.js"; | ||
| import { type ThirdwebContract, getContract } from "../../contract/contract.js"; | ||
| import { sendAndConfirmTransaction } from "../../transaction/actions/send-and-confirm-transaction.js"; | ||
| import { name } from "../common/read/name.js"; | ||
| import { deployERC1155Contract } from "../prebuilts/deploy-erc1155.js"; | ||
| import { balanceOf } from "./__generated__/IERC1155/read/balanceOf.js"; | ||
| import { totalSupply } from "./__generated__/IERC1155/read/totalSupply.js"; | ||
|
|
@@ -37,12 +38,22 @@ describe.runIf(process.env.TW_SECRET_KEY)("TokenERC1155", () => { | |
| chain: ANVIL_CHAIN, | ||
| client: TEST_CLIENT, | ||
| params: { | ||
| name: "Test TokenERC1155", | ||
| name: "Edition", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. Also this tests the exact same thing no? Doesn't actually help coverage
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added back |
||
| contractURI: TEST_CONTRACT_URI, | ||
| }, | ||
| type: "TokenERC1155", | ||
| }); | ||
|
|
||
| expect(contractAddress).toBeDefined(); | ||
| const deployedName = await name({ | ||
| contract: getContract({ | ||
| client: TEST_CLIENT, | ||
| chain: ANVIL_CHAIN, | ||
| address: contractAddress, | ||
| }), | ||
| }); | ||
| expect(deployedName).toBe("Edition"); | ||
|
|
||
| contract = getContract({ | ||
| address: contractAddress, | ||
| chain: ANVIL_CHAIN, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { type Abi, toFunctionSelector } from "viem"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import { ANVIL_CHAIN } from "~test/chains.js"; | ||
| import { TEST_CONTRACT_URI } from "~test/ipfs-uris.js"; | ||
| import { TEST_CLIENT } from "~test/test-clients.js"; | ||
| import { TEST_ACCOUNT_B } from "~test/test-wallets.js"; | ||
| import { resolveContractAbi } from "../../contract/actions/resolve-abi.js"; | ||
| import { type ThirdwebContract, getContract } from "../../contract/contract.js"; | ||
| import { name } from "../../extensions/common/read/name.js"; | ||
| import { sendAndConfirmTransaction } from "../../transaction/actions/send-and-confirm-transaction.js"; | ||
| import { parseNFT } from "../../utils/nft/parseNft.js"; | ||
| import { deployERC721Contract } from "../prebuilts/deploy-erc721.js"; | ||
| import { tokenURI } from "./__generated__/IERC721A/read/tokenURI.js"; | ||
| import { setTokenURI } from "./__generated__/INFTMetadata/write/setTokenURI.js"; | ||
| import { getNFT, isGetNFTSupported } from "./read/getNFT.js"; | ||
| import { mintTo } from "./write/mintTo.js"; | ||
|
|
||
| const client = TEST_CLIENT; | ||
| const chain = ANVIL_CHAIN; | ||
| const account = TEST_ACCOUNT_B; | ||
|
|
||
| let token721Contract: ThirdwebContract; | ||
|
|
||
| describe.runIf(process.env.TW_SECRET_KEY)("deployERC721", () => { | ||
| it("should deploy ERC721 token", async () => { | ||
| const address = await deployERC721Contract({ | ||
| client, | ||
| chain, | ||
| account, | ||
| type: "TokenERC721", | ||
| params: { | ||
| name: "NFTCollection", | ||
| contractURI: TEST_CONTRACT_URI, | ||
| }, | ||
| }); | ||
| expect(address).toBeDefined(); | ||
| token721Contract = getContract({ | ||
| client: TEST_CLIENT, | ||
| chain: ANVIL_CHAIN, | ||
| address, | ||
| }); | ||
| const deployedName = await name({ contract: token721Contract }); | ||
| expect(deployedName).toBe("NFTCollection"); | ||
| }); | ||
|
|
||
| it("should mint an nft when passed an object", async () => { | ||
| const transaction = mintTo({ | ||
| contract: token721Contract, | ||
| nft: { name: "token 0" }, | ||
| to: account.address, | ||
| }); | ||
| await sendAndConfirmTransaction({ | ||
| transaction, | ||
| account, | ||
| }); | ||
|
|
||
| const nft = await getNFT({ contract: token721Contract, tokenId: 0n }); | ||
| expect(nft.metadata.name).toBe("token 0"); | ||
| }); | ||
|
|
||
| it("should mint an nft when passed a string", async () => { | ||
| const transaction = mintTo({ | ||
| contract: token721Contract, | ||
| nft: "ipfs://fake-token-uri", | ||
| to: account.address, | ||
| }); | ||
| await sendAndConfirmTransaction({ | ||
| transaction, | ||
| account, | ||
| }); | ||
| const _uri = await tokenURI({ contract: token721Contract, tokenId: 1n }); | ||
| expect(_uri).toBe("ipfs://fake-token-uri"); | ||
| }); | ||
|
|
||
| it("isGetNFTsSupported should work", async () => { | ||
| const abi = await resolveContractAbi<Abi>(token721Contract); | ||
| const selectors = abi | ||
| .filter((f) => f.type === "function") | ||
| .map((f) => toFunctionSelector(f)); | ||
| expect(isGetNFTSupported(selectors)).toBe(true); | ||
| }); | ||
|
|
||
| it("should return a default value if the URI of the token doesn't exist", async () => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the value of moving these tests around? Feels like unnecessary work. Would you rather focus on actually writing new tests on areas that aren't covered. You can check the uncovered areas on codecov, any file/folder with low coverage is what you should focus on. All of react for example is particularly low
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to put the tests for token standards into the following structures: to speed up the tests in general and (somehow) avoid the issue with out-of-gas + timeout |
||
| /** | ||
| * mint a token, then purposefully change that token's URI to an empty string, using setTokenURI | ||
| */ | ||
| await sendAndConfirmTransaction({ | ||
| transaction: setTokenURI({ | ||
| contract: token721Contract, | ||
| tokenId: 1n, | ||
| // Need to have some spaces because NFTMetadata.sol does not allow to update an empty value | ||
| uri: " ", | ||
| }), | ||
| account, | ||
| }); | ||
|
|
||
| expect( | ||
| await getNFT({ contract: token721Contract, tokenId: 1n }), | ||
| ).toStrictEqual( | ||
| parseNFT( | ||
| { | ||
| id: 1n, | ||
| type: "ERC721", | ||
| uri: "", | ||
| }, | ||
| { | ||
| tokenId: 1n, | ||
| tokenUri: "", | ||
| type: "ERC721", | ||
| owner: null, | ||
| }, | ||
| ), | ||
| ); | ||
| }); | ||
| }); | ||
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.
Can You still pass a contract uri here? Was useful to skip the ipsy upload
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.
oops
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.
added back