inspector: Fix failing test#8019
inspector: Fix failing test#8019eugeneo wants to merge 1 commit intonodejs:masterfrom eugeneo:failing_test
Conversation
test/cctest/test_inspector_socket.cc
Outdated
There was a problem hiding this comment.
This looked okay to me? reinterpret_cast<…>() is an argument to uv_is_active, not EXPECT_EQ
There was a problem hiding this comment.
Right. I reverted the change.
|
nit: since this is fixing a test, the commit message should probably use the 'test: ' prefix. Otherwise LGTM. CI: https://ci.nodejs.org/job/node-test-pull-request/3582/. I have verified locally that this fixes #8006. /cc @bnoordhuis as well. |
|
Thank you for the review. I updated the commit message. |
|
LGTM but can you update the commit log status line to e.g. I admit I don't understand why the test passes for me locally with and without this change. |
Test was updated to wait till the inspector processes socket closure. Fixes: #8006
|
@bnoordhuis I updated the message. This test failure was intermittent - it looks like sockets closing on Mac is somehow more "async" then on other systems (or simply slower) so this test failed because the cleanup code saw inspector socket as still open. If I added a delay (e.g. printed a message to stderr) the test succeeded. |
|
Landed as 49e473a. |
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
This change only touches inspector test.
Description of change
Test was updated to wait till the inspector processes socket closure.
CC: @ofrobots