Skip to content

Eliminate console error when running test - #1179

Merged
Gudahtt merged 3 commits into
mainfrom
eliminate-network-client-queued-request-test-console-error
Apr 14, 2023
Merged

Eliminate console error when running test#1179
Gudahtt merged 3 commits into
mainfrom
eliminate-network-client-queued-request-test-console-error

Conversation

@Gudahtt

Copy link
Copy Markdown
Member

Description

The network client test "queues requests while a previous identical call is still pending..." was emitting a console error due to an unmocked block tracker network call.

That call is now mocked, eliminating the error.

I am unsure why the second call is made during this test, but we suspect that it's because the first block tracker call isn't being cached until after the RPC call that triggered it has been resolved.

To reproduce, use .only on that test and run this command:

yarn workspace @metamask/network-controller jest --coverage=false --verbose=false

Before this change, that command should produce console errors. After this change, there will be none (though there are still lots of warnings).

Changes

N/A

References

This relates to #1178

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

The network client test "queues requests while a previous identical
call is still pending..." was emitting a console error due to an
unmocked block tracker network call.
That call is now mocked, eliminating the error.
I am unsure why the second call is made during this test, but we
suspect that it's because the first block tracker call isn't being
cached until after the RPC call that triggered it has been resolved.
To reproduce, use `.only` on that test and run this command:
`yarn workspace @metamask/network-controller jest --coverage=false --verbose=false`
Before this change, that command should produce console errors. After
this change, there will be none (though there are still lots of
warnings).
This relates to #1178
@Gudahtt
Gudahttforce-pushed the eliminate-network-client-queued-request-test-console-error branch from f4b86aa to 5bb68f8CompareApril 13, 2023 22:06

for (const paramIndex of [...Array(numberOfParameters).keys()]) {
it(`does not reuse the result of a previous request if parameter at index "${paramIndex}" differs`, async () => {// eslint-disable-line
it(`does not reuse the result of a previous request if parameter at index "${paramIndex}" differs`, async () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

My editor removed this when I formatted the document. I'm not sure why it was there in the first place, but it appears to have been unnecessary.

@Gudahtt
Gudahtt marked this pull request as ready for review April 13, 2023 22:54
@Gudahtt
Gudahtt requested a review from a team as a code ownerApril 13, 2023 22:54
@Gudahtt

Copy link
Copy Markdown
MemberAuthor

I found one more error in the equivalent test for methods that do have a block param.

I have verified locally that these are the only unmocked network calls in the network controller tests.

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt
Gudahtt merged commit e897962 into mainApr 14, 2023
@Gudahtt
Gudahtt deleted the eliminate-network-client-queued-request-test-console-error branch April 14, 2023 16:39
@legobeatlegobeat mentioned this pull request Apr 25, 2023
Sign up for freeto 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.

2 participants

@Gudahtt@mcmire