This repository has been archived by the owner on Sep 11, 2020. It is now read-only.
Add close method for bcoin wallet #176
Merged
Conversation
This file contains 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
luckysori
added a commit
to comit-network/comit-rs
that referenced
this pull request
Mar 28, 2020
WIP because there's currently a `yalc` local dependency which will be replaced once comit-network/comit-js-sdk#176 is merged and `comit-js-sdk` is released with its changes. We ensure that all nodes are stopped and all wallets are closed during Jest teardown.
bonomat
suggested changes
Mar 29, 2020
thomaseizinger
approved these changes
Mar 29, 2020
Otherwise the handle remains open until the timeout promise resolves, which can make certain programs hang unexpectedly.
da-kami
approved these changes
Mar 30, 2020
|
bors r+ |
Build succeeded |
bors bot
added a commit
to comit-network/comit-rs
that referenced
this pull request
Mar 30, 2020
2330: Close all handles during Jest teardown r=bonomat a=luckysori _Waiting for comit-network/comit-js-sdk#176 to be merged and for those changes to be included in a release of `comit-js-sdk` before we can update the last commit in this PR to use that release instead of a local dependency created via [`yalc`](https://github.com/whitecolor/yalc) (which seems to work **much** better than `yarn link`)._ Fixes #2211. 1. Turns out Jest will hang until every timeout is cleared, so racing against a timeout that is only cleared after it resolves was not good. 2. The proper teardown of the e2e test environment is fully achieved with 3698f3a, after a lot of debugging. Unfortunately, Jest recommends this ``` Consider running Jest with `--detectOpenHandles` to troubleshoot this issue ``` when tests hang after passing, but it just [doesn't seem to do anything](jestjs/jest#9473). I tried using [`leaked-handles`](https://github.com/Raynos/leaked-handles) without success, but finally found [`wtfnode`](https://github.com/myndzi/wtfnode), which identified `bcoin` as the culprit. --- My OCD was definitely triggered with this one, as a different error got me to investigate how to get rid of `smack-my-jasmine-up`. Turns out removing it did not fix that other error, but I still see it as a win, since depending on such an obscure library is bad. --- And another bug squashed! No more ``` MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGHUP listeners added to [process]. Use emitter.setMaxListeners() to increase limit ``` errors, by combining cde8e0c and 127a4a2: - shutting down the logger during teardown. - closing log files after 5 seconds of inactivity, to prevent us from reaching the limit of listeners. I believe log files can be reopened if something else needs to be logged. Co-authored-by: Lucas Soriano del Pino <l.soriano.del.pino@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
These are a couple of patches that need to be released before comit-network/comit-rs#2330 can be merged in.