Add unit tests for incoming transaction block events#9755
Merged
Conversation
d55dfd8 to
a0a2dcb
Compare
e187005 to
a37c968
Compare
Member
Author
|
|
a37c968 to
fea2e1a
Compare
Collaborator
Builds ready [fea2e1a]
Page Load Metrics (402 ± 70 ms)
|
Base automatically changed from
incoming-transaction-tests-replace-shared-mocks
to
develop
October 29, 2020 15:16
fea2e1a to
6843e38
Compare
Collaborator
Builds ready [6843e38]
Page Load Metrics (426 ± 61 ms)
|
rekmarks
reviewed
Oct 29, 2020
rekmarks
requested changes
Oct 29, 2020
Member
rekmarks
left a comment
There was a problem hiding this comment.
These tests look good, I mainly have some questions/nits. My review should not be considered final, and I have a significant amount of work to do to fully understand the incoming transactions controller. It'd be great if someone with more context could do a full pass.
One general requested change for the test file:
We should update all existing assert.equal and deepEqual calls to their strict equivalents.
Unit tests have been added to the incoming transactions controller to ensure that block updates are correctly resulting in state updates when incoming transactions are enabled. All other events that trigger state updates are tested as well. The tests were written to be minimally dependent upon implementation details of the controller itself. `nock` was used to mock the API response from Etherscan. Each event is triggered asynchronously by `sinon`, as in production they are likely only triggered asynchronously. This was extracted from #9583 This PR includes a new `waitUntilCalled` module meant to help with writing asynchronous tests. It allows you to wait until a stub has been called.
6843e38 to
28a3d43
Compare
Collaborator
Builds ready [28a3d43]
Page Load Metrics (438 ± 73 ms)
|
Gudahtt
added a commit
that referenced
this pull request
Oct 29, 2020
This comment block describes the responsibilities of this controller. This was motivated by a suggestion made during review of #9755 [1] [1]: #9755 (comment)
rekmarks
approved these changes
Oct 30, 2020
Gudahtt
added a commit
that referenced
this pull request
Oct 30, 2020
This comment block describes the responsibilities of this controller. This was motivated by a suggestion made during review of #9755 [1] [1]: #9755 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unit tests have been added to the incoming transactions controller to ensure that block updates are correctly resulting in state updates when incoming transactions are enabled. All other events that trigger state updates are tested as well.
The tests were written to be minimally dependent upon implementation details of the controller itself.
nockwas used to mock the API response from Etherscan. Each event is triggered asynchronously bysinon, as in production they are likely only triggeredasynchronously.
This was extracted from #9583
This PR includes a new
waitUntilCalledmodule meant to help with writing asynchronous tests. It allows you to wait until a stub has been called.