This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
Missing test case for worker's Worker.prototype.destroy in tests suite #8223
Labels
Comments
|
Hm... want to submit a PR for this? |
misterdjules
pushed a commit
to misterdjules/node
that referenced
this issue
Sep 2, 2014
Add a simple test to cover workers' implementation of Worker.prototype.destroy(). Before adding this test, this code wouldn't be covered by the tests suite, and any regression introduced in workers' implementation of Worker.prototype.destroy wouldn't be caught. Fixes nodejs#8223.
|
@indutny Done. Please let me know if you have any question. |
misterdjules
pushed a commit
to misterdjules/node
that referenced
this issue
Sep 2, 2014
Add a simple test to cover workers' implementation of Worker.prototype.destroy(). Before adding this test, this code wouldn't be covered by the tests suite, and any regression introduced in workers' implementation of Worker.prototype.destroy wouldn't be caught. Fixes nodejs#8223.
trevnorris
pushed a commit
that referenced
this issue
Sep 18, 2014
Add a simple test to cover workers' implementation of Worker.prototype.destroy(). Before adding this test, this code wouldn't be covered by the tests suite, and any regression introduced in workers' implementation of Worker.prototype.destroy wouldn't be caught. Fixes: #8223 Reviewed-by: Trevor Norris <trev.norris@gmail.com>
|
Fixed by 9c992bd. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
I got bitten by the lack of coverage in the worker implementation of
Worker.prototype.destroy. I had submitted a PR with a 100% repro bug (a typo in a call to a function that would always result in an "undefined function called" exception) in the worker implementation ofWorker.prototype.destroy.Running
make testdidn't catch the problem. Although it seems this API is not used internally, and even if callingdestroyfrom within a cluster worker may seem rare, It's likely that at least some users are doing it. Merging the above mentioned PR would have broken this API for these users.I suggest adding at least one test case for this API in the tests suite, and if this API should not be exposed in the future, maybe plan for its deprecation.
The text was updated successfully, but these errors were encountered: