Skip to content
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

test: add test for fee-estimate fix #1506

Merged
merged 1 commit into from
Oct 9, 2024
Merged

test: add test for fee-estimate fix #1506

merged 1 commit into from
Oct 9, 2024

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented Oct 9, 2024

Description

Adds a test that covers the case related to the recent fix (#1505) in the fee-estimate endpoint. The test is mocked at block 22887036 and with Polkadot registry v1003000.

How to test

Run yarn test or yarn test transactionFeeEstimateService and all tests should pass.

@Imod7 Imod7 requested a review from a team as a code owner October 9, 2024 07:55
Copy link
Contributor

@filvecchiato filvecchiato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! LGTM

@@ -71,6 +105,17 @@ describe('TransactionFeeEstimateService', () => {
).toStrictEqual(validRuntimeResponse);
});

it('Works with a valid transaction at block 22887036', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transactionPaymentApi.queryInfo doesn't care of the block number, it is always queried against the current runtime version. So being strict, we don't need here the detail at what block it should work.

Copy link
Contributor Author

@Imod7 Imod7 Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this very good point!
In production environment, yes you are absolutely right, the transactionPaymentApi.queryInfo call will work against the finalized block (related line of code here). However in the test environment, I cannot (and I should not) query the live chain and get the finalized block. That is why I need to specify a block height manually and pass it as an argument which is what happens here.
Manually setting all these different parameters in tests is expected because we want to have control of the setting in which the tests are run. That is why we mock everything (or what we need) and we can simulate being at a specific block height using that specific type registry and metadata, while also mocking the expected response.

@Imod7 Imod7 merged commit c365490 into master Oct 9, 2024
15 checks passed
@Imod7 Imod7 deleted the domi-queryInfo-test branch October 9, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants