test-repl-tab-complete.js "params and { on separate line" fails silently. #21586
Comments
Previously, the code displayed properties backwards (e.g., showing prototype properties before own properties). It also did uniqueness checks during this processing, so these checks were done backwards. After this change, the properties continue to be displayed backwards, but the uniqueness checks are done in the proper order. Fixes: nodejs#15199 See also: nodejs#21586 which was discovered during the testing of this fix.
devsnek
added
repl
|
@nodejs/repl |
Previously, the code displayed properties backwards (e.g., showing prototype properties before own properties). It also did uniqueness checks during this processing, so these checks were done backwards. After this change, the properties continue to be displayed backwards, but the uniqueness checks are done in the proper order. See also: #21586 which was discovered during the testing of this fix. Fixes: #15199 PR-URL: #21588 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Previously, the code displayed properties backwards (e.g., showing prototype properties before own properties). It also did uniqueness checks during this processing, so these checks were done backwards. After this change, the properties continue to be displayed backwards, but the uniqueness checks are done in the proper order. See also: #21586 which was discovered during the testing of this fix. Fixes: #15199 PR-URL: #21588 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
There seem to be two things going on here. First, the assertion is wrong. It shouldn't be no results. It should get Second, when the test fails, nothing happens. The callback is running, but even throwing in the callback doesn't result in the error failing in this test case and a few others. Not sure why it's different. Even if I comment out all other test cases, it still exhibits this behavior. |
|
Maybe domains are messing with the error handling? Maybe the REPL completer is creating a nested REPL and that contributes to the problem? |
In Node.js 8.x, completion returned nothing and the test was correct. In Node.js 10.x, completion returns In some situations, the error is handled by the domain of a nested REPL so we never see it... |
|
Definitely looks like it's a bug in the REPL where domains cause the nested REPL to swallow errors that should be transmitted back to the parent REPL. I've got a test for this bug and I think I have a fix that I'm testing right now. |
For tab completion, a REPLServer instance will sometimes create another REPLServer instance. If a callback is sent to the `.complete()` function and that callback throws an error, it will be swallowed by the nested REPLs domain. Re-throw the error so that processes don't silently exit without any indication of an error (including a status code). Fixes: nodejs#21586
|
Proposed fix: #23004 |
For tab completion, a REPLServer instance will sometimes create another REPLServer instance. If a callback is sent to the `.complete()` function and that callback throws an error, it will be swallowed by the nested REPLs domain. Re-throw the error so that processes don't silently exit without any indication of an error (including a status code). Fixes: nodejs#21586 PR-URL: nodejs#23004 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
Fixed in 83d0404 |
In
test/parallel/test-repl-tab-complete.js, the following not only fails silently, it aborts the running of the remainder of the tests in this source file:If this test is changed to have only assertions that pass (e.g.,
assert.strictEqual(null, null);, then execution continues. If it has assertions that fail (e.g.,assert.strictEqual(1, 2);, then the test fails silently, the remainder of the tests are aborted, and overall success is reported.The text was updated successfully, but these errors were encountered: