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

Update new tests to destroy block tracker #156

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Oct 5, 2022

Certain middleware use a polling block tracker to retrieve the latest block. This type of block tracker has the ability to make a quick request in addition to polling, but doing so requires starting the poll loop and then immediately stopping it. This means that it is possible for the block tracker to be in a running state when a test that uses it ends. To avoid this, we need to make sure we completely shut down the block tracker at the end of each test. The ability to do this was added in a recent version of eth-block-tracker.

Certain middleware use a polling block tracker to retrieve the latest
block. This type of block tracker has the ability to make a quick
request in addition to polling, but doing so requires starting the poll
loop and then immediately stopping it. This means that it is possible
for the block tracker to be in a running state when a test that uses it
ends. To avoid this, we need to make sure we completely shut down the
block tracker at the end of each test. The ability to do this was added
in a recent version of `eth-block-tracker`.
@mcmire
Copy link
Contributor Author

mcmire commented Oct 5, 2022

Putting this in a draft state because this PR is dependent upon MetaMask/eth-block-tracker#117 (otherwise we can't grab the Provider type from eth-block-tracker).

@Gudahtt
Copy link
Member

Gudahtt commented Jan 30, 2023

@mcmire this seems to be unblocked now

@mcmire
Copy link
Contributor Author

mcmire commented Jan 30, 2023

@Gudahtt Hmm, these changes don't seem as useful as I thought. After running the test suite on this branch, I'm seeing the "Jest did not exit one second after the test run has completed" message from Jest, which I feel like these changes were intended to remove. I've played around with this a bit, and it seems that destroying the PollingBlockTracker doesn't kill the last setTimeout call it makes when it advances to the next iteration of the polling loop. We can "fix" this by instantiating PollingBlockTracker with keepEventLoopActive: false, but that papers over the problem in my opinion. So I'm going to keep this open until eth-block-tracker is properly fixed. In the meantime I've created MetaMask/eth-block-tracker#128.

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