clean and migrate callback erc1154 UT file part 1 #145
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #145 +/- ##
========================================
Coverage 84.58% 84.58%
========================================
Files 35 35
Lines 1077 1077
Branches 221 221
========================================
Hits 911 911
Misses 166 166 ☔ View full report in Codecov by Sentry. |
| const task = await iexecPoco.viewTask(taskId); | ||
| await time.setNextBlockTimestamp(task.revealDeadline); |
There was a problem hiding this comment.
| const task = await iexecPoco.viewTask(taskId); | |
| await time.setNextBlockTimestamp(task.revealDeadline); | |
| const task = await iexecPoco.viewTask(taskId); | |
| expect(task.status).to.equal(TaskStatusEnum.REVEALING); | |
| expect(task.revealCounter).to.equal(1); | |
| // caller is scheduler, task status is revealing, before final deadline, | |
| // reveal counter is reached | |
| // but resultsCallback is bad |
| const { resultsCallback } = buildResultCallbackAndDigest(567); | ||
| await expect( | ||
| iexecPocoAsScheduler.finalize(taskId, results, resultsCallback), | ||
| ).to.be.revertedWithoutReason(); |
There was a problem hiding this comment.
| ).to.be.revertedWithoutReason(); | |
| ).to.be.revertedWithoutReason(); // require#4 (part 2) |
| ).to.be.revertedWithoutReason(); | ||
| }); | ||
|
|
||
| it('Should finalize task when result callback is invalid', async () => { |
There was a problem hiding this comment.
Why do you say result callback is invalid?
There was a problem hiding this comment.
Here the invalid callback is because callback: appAddress, and should have been callback: oracleConsumerInstance.address,
It's to mimic the test incallback.js => describe('invalid callback', async () => {
https://github.com/iExecBlockchainComputing/PoCo/blob/feature/migrate-erc1154-UT/test/ERC1154/callback.js.skip#L501
| .to.not.emit(oracleConsumerInstance, 'GotResult'); | ||
| }); | ||
|
|
||
| it('Should finalize task when callback is EOA', async () => { |
There was a problem hiding this comment.
I would move this closer to "should tests" maybe below "Should finalize task of deal payed by requester (no callback, no dataset)" (?)
There was a problem hiding this comment.
Updated here
| .reveal(taskId, callbackResultDigest) | ||
| .then((tx) => tx.wait()); | ||
| const task = await iexecPoco.viewTask(taskId); | ||
| await time.setNextBlockTimestamp(task.revealDeadline); |
There was a problem hiding this comment.
Why do you fly to reveal deadline?
There was a problem hiding this comment.
removed here
Co-authored-by: Jérémy James Toussaint <33313130+jeremyjams@users.noreply.github.com>
| assets: ordersAssets, | ||
| requester: requester.address, | ||
| prices: ordersPrices, | ||
| callback: appAddress, // Non-EIP1154 contract |
There was a problem hiding this comment.
Maybe use another random address to avoid confusion.
There was a problem hiding this comment.
here the appAddress was to meant to have a smart contract address and not a EOA without code in it
There was a problem hiding this comment.
How about that
PoCo/test/byContract/IexecPoco/IexecPoco1.test.ts
Lines 121 to 124 in 4821374
Or at least add a comment to explain than it's a random contract.
No description provided.