FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix flaky inspector-cli tests when breakpoints are restored by Trott · Pull Request #38431 · nodejs/node · GitHub

/ node Public

test: fix flaky inspector-cli tests when breakpoints are restored - #38431

Merged
Trott merged 1 commit into
nodejs:masterfrom
Trott:pid-fix
May 3, 2021
ConversationCommits (1)ChecksFiles changed
Merged

test: fix flaky inspector-cli tests when breakpoints are restored#38431
Trott merged 1 commit into
nodejs:masterfrom
Trott:pid-fix

Conversation

Trott commented Apr 27, 2021

Copy link
Copy Markdown
Member

Sometimes, there isn't a newine in the AIX output that already has a
comment indicating it needs investigation.

github-actions Bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 27, 2021

Trott commented Apr 27, 2021

Copy link
Copy Markdown
Member Author

Refs: https://ci.nodejs.org/job/node-test-commit-aix/36437/nodes=aix72-ppc64/console

00:30:58 not ok 3190 inspector-cli/test-inspector-cli-pid
00:31:08   ---
00:31:08   duration_ms: 10.872
00:31:08   severity: fail
00:31:09   exitcode: 1
00:31:09   stack: |-
00:31:09     node:internal/process/promises:246
00:31:09               triggerUncaughtException(err, true /* fromPromise */);
00:31:09               ^
00:31:09     
00:31:09     AssertionError [ERR_ASSERTION]: ifError got unwanted exception: Timeout (10000) while waiting for />\s+(?:\n1 breakpoints restored\.)?$/; found: Warning: script 'alive.js' was not loaded yet.
00:31:09     debug> 
00:31:09     break in test/fixtures/inspector-cli/alive.js:3
00:31:09       1 let x = 0;
00:31:09       2 function heartbeat() {
00:31:09     > 3   ++x;
00:31:09       4 }
00:31:09       5 setInterval(heartbeat, 50);
00:31:09     debug> 1 breakpoints restored.
00:31:09     
00:31:09         at Timeout.<anonymous> (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64/test/common/inspector-cli.js:84:18)
00:31:09         at listOnTimeout (node:internal/timers:557:17)
00:31:09         at processTimers (node:internal/timers:500:7) {
00:31:09       generatedMessage: false,
00:31:09       code: 'ERR_ASSERTION',
00:31:09       actual: Error: Timeout (10000) while waiting for />\s+(?:\n1 breakpoints restored\.)?$/; found: Warning: script 'alive.js' was not loaded yet.
00:31:09       debug> 
00:31:09       break in test/fixtures/inspector-cli/alive.js:3
00:31:09         1 let x = 0;
00:31:09         2 function heartbeat() {
00:31:09       > 3   ++x;
00:31:09         4 }
00:31:09         5 setInterval(heartbeat, 50);
00:31:09       debug> 1 breakpoints restored.
00:31:09       
00:31:09           at Timeout.<anonymous> (/home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64/test/common/inspector-cli.js:84:18)
00:31:09           at listOnTimeout (node:internal/timers:557:17)
00:31:09           at processTimers (node:internal/timers:500:7),
00:31:09       expected: null,
00:31:09       operator: 'ifError'
00:31:09     }
00:31:09   ...

Trott added aix Issues and PRs related to the AIX platform. flaky-test Issues and PRs related to the tests with unstable failures on the CI. inspector Issues and PRs related to the V8 inspector protocol labels Apr 27, 2021

nodejs-github-bot commented Apr 27, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/37750/

RaisinTen reviewed Apr 27, 2021
View reviewed changes
Comment thread test/inspector-cli/test-inspector-cli-pid.js Outdated

RaisinTen Apr 27, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
// What we're diong for now:
// What we're doing for now:

nit: typo from an earlier change

richardlau commented Apr 27, 2021

Copy link
Copy Markdown
Member

Is there perhaps some deeper underlying issue with the inspector-cli tests?
Today's daily failed in inspector_cli_test_inspector_cli_address: https://ci.nodejs.org/job/node-test-commit-aix/36460/nodes=aix72-ppc64/testReport/junit/(root)/test/inspector_cli_test_inspector_cli_address/

Trott commented Apr 27, 2021

Copy link
Copy Markdown
Member Author

Is there perhaps some deeper underlying issue with the inspector-cli tests?
Today's daily failed in inspector_cli_test_inspector_cli_address: https://ci.nodejs.org/job/node-test-commit-aix/36460/nodes=aix72-ppc64/testReport/junit/(root)/test/inspector_cli_test_inspector_cli_address/

Yeah, I think it's all one issue that:

  • Should be addressed in waitPrompt() in test/common/inspector-cli.js rather than done piecemeal throughout the files. I'll look at opening a PR to do that.
  • Doesn't really affect end users and might not even really be considered a bug. I'm guessing it has to do with buffering output somewhere.

I've been wondering if it makes sense to differentiate stdout and stderr in the debugger. The prompt should go to stdout, I imagine, but the warning messages should go to stderr perhaps. If we then keep them separate in the tests, it should be easy to make them robust.

Trott commented Apr 29, 2021

Copy link
Copy Markdown
Member Author

OK, I've generalized the workaround. PTAL

nodejs-github-bot commented Apr 29, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/37791/

nodejs-github-bot commented Apr 29, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/37804/

nodejs-github-bot commented May 3, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/37890/

Trott changed the title test: fix flaky inspector-cli/test-inspector-cli-pid on AIX test: fix flaky inspector-cli tests when breakpoints are restored May 3, 2021
test: fix flaky inspector-cli tests when breakpionts are restored
18e4f40 
PR-URL: nodejs#38431
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott merged commit 18e4f40 into nodejs:master May 3, 2021

Trott commented May 3, 2021

Copy link
Copy Markdown
Member Author

Landed in 18e4f40

Trott deleted the pid-fix branch May 3, 2021 16:00
targos pushed a commit that referenced this pull request May 17, 2021
test: fix flaky inspector-cli tests when breakpionts are restored
e64ebac 
PR-URL: #38431
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos mentioned this pull request May 18, 2021
Merged
Trott mentioned this pull request Jun 5, 2021
Closed
bl-ue mentioned this pull request Jun 12, 2021
Closed
tniessen mentioned this pull request Jul 7, 2021
Merged
richardlau pushed a commit that referenced this pull request Jul 19, 2021
test: fix flaky inspector-cli tests when breakpionts are restored
7f2017d 
PR-URL: #38431
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

richardlau commented Jul 19, 2021

Copy link
Copy Markdown
Member

This didn't seem to be included in #38858 but does cherry-pick cleanly across to v14.x-staging after that landed.

richardlau pushed a commit that referenced this pull request Jul 20, 2021
test: fix flaky inspector-cli tests when breakpionts are restored
99fd8bf 
PR-URL: #38431
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau mentioned this pull request Jul 20, 2021
Merged
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

RaisinTen RaisinTen left review comments
jasnell jasnell approved these changes
richardlau richardlau approved these changes
gireeshpunathil gireeshpunathil approved these changes

Assignees

No one assigned

Labels

aix Issues and PRs related to the AIX platform. flaky-test Issues and PRs related to the tests with unstable failures on the CI. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL