Skip to content

Eliminate console error when running test#1179

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

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

Conversation

@Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Apr 13, 2023

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 Gudahtt force-pushed the eliminate-network-client-queued-request-test-console-error branch from f4b86aa to 5bb68f8 Compare April 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
Member Author

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 owner April 13, 2023 22:54
@Gudahtt
Copy link
Member Author

Gudahtt commented Apr 14, 2023

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.

Copy link
Contributor

@mcmire mcmire left a comment

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 main Apr 14, 2023
@Gudahtt Gudahtt deleted the eliminate-network-client-queued-request-test-console-error branch April 14, 2023 16:39
@legobeat legobeat mentioned this pull request Apr 25, 2023
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Eliminate console error when running test

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

* Eliminate error in second test
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Eliminate console error when running test

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

* Eliminate error in second test
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.

2 participants