-
Notifications
You must be signed in to change notification settings - Fork 85
feat: create acceptance tests for tx pool feature #4443
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
base: main
Are you sure you want to change the base?
Conversation
…#4398) Signed-off-by: nikolay <[email protected]>
…olService` interface (#4405) Signed-off-by: nikolay <[email protected]>
…#4398) Signed-off-by: nikolay <[email protected]>
…olService` interface (#4405) Signed-off-by: nikolay <[email protected]>
…/transaction-pool
Signed-off-by: nikolay <[email protected]> Signed-off-by: Simeon Nakov <[email protected]> Co-authored-by: Simeon Nakov <[email protected]>
…/transaction-pool
Signed-off-by: nikolay <[email protected]> Signed-off-by: Konstantina Blazhukova <[email protected]> Co-authored-by: Nikolay Atanasow <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: nikolay <[email protected]>
Test Results 20 files ± 0 263 suites ±0 18m 21s ⏱️ - 16m 47s For more details on these failures, see this check. Results for commit cc134c3. ± Comparison against base commit 57c9fa1. ♻️ This comment has been updated with latest results. |
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]> Signed-off-by: nikolay <[email protected]> Co-authored-by: Konstantina Blazhukova <[email protected]>
…s-for-tx-pool-feature
Signed-off-by: nikolay <[email protected]> Signed-off-by: Konstantina Blazhukova <[email protected]> Co-authored-by: Konstantina Blazhukova <[email protected]>
…s-for-tx-pool-feature
Signed-off-by: nikolay <[email protected]>
# Conflicts: # packages/relay/src/lib/precheck.ts
…s-for-tx-pool-feature
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
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.
few comments and questions
describe('Transaction Pool feature', async () => { | ||
describe('ENABLE_TX_POOL = true', async () => { | ||
overrideEnvsInMochaDescribe({ ENABLE_TX_POOL: true }); | ||
it('should have equal nonces (pending and latest) after successful validated transaction', async () => { |
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.
nit: successfully* ?
const txHash = await relay.sendRawTransaction(signedTx); | ||
await relay.pollForValidTransactionReceipt(txHash); | ||
|
||
const nonceLatest = await relay.getAccountNonce(accounts[1].address); |
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.
maybe lets add a comment clarifying that they are equal because pending will get only the MN value and pending pool would be empty? wdy
expect(nonceLatest).to.equal(noncePending); | ||
}); | ||
|
||
it('scenario #5', async () => {}); |
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.
is this some leftover?
}); | ||
const txHash2 = await relay.sendRawTransaction(signedTx2); | ||
|
||
const [receipt1, receipt2] = await Promise.all([ |
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.
Do we need this here, we are essentially checking the nonces before we get the transaction receipts, why do we care about the status? For me this was kind of confusing, cause i thought we would wait for the receipts then check the nonces. Also side question the latest nonce after the receipt should be the same as the pending correct?
type: 1, | ||
}; | ||
|
||
describe('Transaction Pool feature', async () => { |
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.
I assume these tests are with USE_ASYNC_TX_PROCESSING set to true, should we have some with the value set to false?
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #4443 +/- ##
==========================================
- Coverage 96.24% 95.88% -0.36%
==========================================
Files 121 126 +5
Lines 19941 20310 +369
Branches 1755 1761 +6
==========================================
+ Hits 19192 19475 +283
- Misses 727 811 +84
- Partials 22 24 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Description
Create acceptance tests for the feature based on the Acceptance Criteria created when designing the feature.
Related issue(s)
Fixes #4396
Testing Guide
Changes from original design (optional)
N/A
Additional work needed (optional)
N/A
Checklist