Skip to content

test: fix flaky test-inspector-connect-main-thread - #29588

Closed
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:fix-flaky-inspector-connect
Closed

test: fix flaky test-inspector-connect-main-thread#29588
addaleax wants to merge 2 commits into
nodejs:masterfrom
addaleax:fix-flaky-inspector-connect

Conversation

@addaleax

Copy link
Copy Markdown
Member

Using console.log() likely interferes with the functionality of the
test, which also checks the interaction between inspector
and console.log() as part of the test. Using process._rawDebug()
solves that issue.

Refs: #28870
Refs: #29582

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

zaverdenand others added 2 commits September 16, 2019 21:13
It fix 2 issues in provided Loader hooks examples:
1. Original ``new URL(`${process.cwd()}/`, 'file://');``
is not cross-platform, it gives wrong URL on windows
2. Based on `CHECK` in ModuleWrap::Resolve (node 12.9.1,
https://github.com/nodejs/node/blob/v12.9.1/src/module_wrap.cc#L1132)
the 2nd parameter should be a `string`, not an `URL` object
PR-URL: nodejs#29373
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Using `console.log()` likely interferes with the functionality of the
test, which also checks the interaction between inspector
and `console.log()` as part of the test. Using `process._rawDebug()`
solves that issue.
Refs: nodejs#28870
Refs: nodejs#29582
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2019
@addaleaxaddaleax added inspector Issues and PRs related to the V8 inspector protocol worker Issues and PRs related to Worker support. labels Sep 17, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@addaleaxaddaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 17, 2019
@addaleax

Copy link
Copy Markdown
MemberAuthor

CI is green, 👍 here to approve fast-tracking

@Trott

Copy link
Copy Markdown
Member

Landed in 3adec43

@TrottTrott closed this Sep 17, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Sep 17, 2019
Using `console.log()` likely interferes with the functionality of the
test, which also checks the interaction between inspector
and `console.log()` as part of the test. Using `process._rawDebug()`
solves that issue.
Refs: nodejs#28870
Refs: nodejs#29582
PR-URL: nodejs#29588
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@addaleax
addaleax deleted the fix-flaky-inspector-connect branch September 17, 2019 19:40
targos pushed a commit that referenced this pull request Sep 20, 2019
Using `console.log()` likely interferes with the functionality of the
test, which also checks the interaction between inspector
and `console.log()` as part of the test. Using `process._rawDebug()`
solves that issue.
Refs: #28870
Refs: #29582
PR-URL: #29588
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BridgeARBridgeAR mentioned this pull request Sep 24, 2019
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
Using `console.log()` likely interferes with the functionality of the
test, which also checks the interaction between inspector
and `console.log()` as part of the test. Using `process._rawDebug()`
solves that issue.
Refs: #28870
Refs: #29582
PR-URL: #29588
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast-trackPRs that do not need to wait for 48 hours to land.inspectorIssues and PRs related to the V8 inspector protocoltestIssues and PRs related to the tests.workerIssues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@addaleax@nodejs-github-bot@Trott@targos@cjihrig@devnexen@zaverden