IPAsset - Batch Integration Tests #509
Conversation
bpolania
requested review from
LeoHChen,
DonFungible,
edisonz0718,
jacob-tucker,
AndyBoWu,
allenchuang,
bonnie57 and
DracoLi
as code owners
| const expect = chai.expect; | ||
|
|
||
| const pool = evenSplitGroupPoolAddress[aeneid]; | ||
| const walletAddress = process.env.TEST_WALLET_ADDRESS! as Address; |
There was a problem hiding this comment.
Minor thing. In the integration/util/util file, there is a variable called TEST_WALLET_ADDRESS. You can directly import it.
| isPublicMinting: true, | ||
| mintOpen: true, | ||
| contractURI: "test-uri", | ||
| mintFeeRecipient: walletAddress, |
There was a problem hiding this comment.
Could we add a mintFee property? I think we need to consider the complex situation regarding the fee. There are many logical tests we can test if we add the mint fee.
| walletClient, | ||
| erc20Address[aeneid] | ||
| ); | ||
| await mockERC20.mint(walletAddress, 100000n); |
There was a problem hiding this comment.
It's the same. We need to consider the WIP token instead of ERC20, which is the common case.
| const batchRequest = { | ||
| requests: requests.map(req => ({ | ||
| ...req, | ||
| })), |
There was a problem hiding this comment.
You don't need to map it. You can write it as follows:
const batchRequest = {
requests,
};
| const batchResult = await client.ipAsset.batchRegisterIpAssetsWithOptimizedWorkflows(batchRequest); | ||
| expect(batchResult.registrationResults).to.be.an("array"); | ||
| expect(batchResult.registrationResults.length).to.be.lessThan(requests.length); | ||
|
|
There was a problem hiding this comment.
I think we need to provide an exact number instead of a rough estimate. It will provide exact feedback.
| })); | ||
|
|
||
| const totalPercentage = maxRecipientsDistribution.reduce((sum, item) => sum + item.percentage, 0); | ||
| expect(totalPercentage).to.equal(100); |
There was a problem hiding this comment.
It's your own logic, we don't need to assert.
| } | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
How to verify royalty token ownership? I don't see the related assertion.
|
|
||
| describe("Batch IP Registration with Royalty Distribution", () => { | ||
| let client: StoryClient; | ||
| let nftContract: Hex; |
There was a problem hiding this comment.
Could you rename the spgNftContract? Because in the IPAsset module, there are two kinds of NFT contracts, namely SPG NFT contract and NFT contract.
bpolania
changed the title
Description
This PR adds integration tests for batch register IP functionality