E2E Testing fixes and cleanup #1765
Conversation
| @@ -60,16 +62,6 @@ describe("Capacity Transactions", function () { | |||
| assert.notEqual(schemaId, undefined, "setup should populate schemaId"); | |||
| }); | |||
|
|
|||
| async function assertAddNewKey(capacityKeys: KeyringPair, addKeyPayload: AddKeyData, newControlKeypair: KeyringPair) { | |||
| @@ -240,8 +232,7 @@ describe("Capacity Transactions", function () { | |||
| page_size: 999 | |||
| } | |||
| ); | |||
| const response: MessageResponse = get.content[get.content.length - 1]; | |||
| assert.equal(response.payload, "0xdeadbeef", "payload should be 0xdeadbeef"); | |||
| assertHasMessage(get, x => x.payload.isSome && x.payload.toString() === "0xdeadbeef"); | |||
| @@ -254,8 +245,6 @@ describe("Capacity Transactions", function () { | |||
| before(async function () { | |||
| capacityKeys = createKeys("CapacityKeys"); | |||
| capacityProvider = await createMsaAndProvider(fundingSource, capacityKeys, "CapacityProvider", FUNDS_AMOUNT); | |||
| }) | |||
| beforeEach(async function () { | |||
| await assert.doesNotReject(stakeToProvider(fundingSource, capacityKeys, capacityProvider, amountStaked)); | |||
There was a problem hiding this comment.
No need to stake before each
| assert.fail("batchAll should have caused an error"); | ||
| } catch (err) { | ||
| error = err; | ||
| assert.notEqual(error, undefined, " batchAll should return an error"); | ||
| assert.notEqual(err, undefined, " batchAll should return an error"); | ||
| } | ||
| assert.notEqual(error, undefined, " batchAll should return an error"); |
There was a problem hiding this comment.
This is the best way to do an error assertion.
There was a problem hiding this comment.
Just curious, any objection to using assert.rejects?
https://nodejs.org/api/assert.html#assertrejectsasyncfn-error-message
Also, I wonder if at some point (not now) we should switch these to Jest (as all of our other Node projects are using it for testing)?
There was a problem hiding this comment.
Or, if we want to keep using Mocha here, I'd be in favor of eventually swapping out the standard Node assert package for something a bit more capable. But definitely not a priority item.
There was a problem hiding this comment.
Ah right. Assert rejects likely would be better. I was just updating it to still match the existing pattern, but that would be better. Next time.
There was a problem hiding this comment.
Re: Jest
We might be able to, but I wasn't willing to make the switch yet.
| @@ -155,7 +157,7 @@ describe("MSA Key management", function () { | |||
| assert.notEqual(event, undefined, 'should have added public key'); | |||
|
|
|||
| // Cleanup | |||
| await assert.doesNotReject(ExtrinsicHelper.deletePublicKey(keys, thirdKey.publicKey).signAndSend()); | |||
| await assert.doesNotReject(ExtrinsicHelper.deletePublicKey(keys, thirdKey.publicKey).signAndSend('current')); | |||
There was a problem hiding this comment.
Important fix. deletePublicKey is a free transaction, so doesn't use up a nonce.
| await ExtrinsicHelper.api.disconnect(); | ||
| await ExtrinsicHelper.apiPromise.disconnect(); |
There was a problem hiding this comment.
No need to wait for this
Goal
The goal of this PR is the final misc pieces of getting the e2e tests less flaky. Does this fix all of it? No. Most? Yes.
Closes #1731
Discussion