Skip to content

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
Merged

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

Conversation

@Trott

@Trott 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 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

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 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

Copy link
Copy Markdown
Collaborator

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.

Suggested change
// What we're diong for now:
// What we're doing for now:

nit: typo from an earlier change

@richardlau

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

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

Trott commented Apr 29, 2021

Copy link
Copy Markdown
Member Author

OK, I've generalized the workaround. PTAL

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Trott 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
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
Trott merged commit 18e4f40 into nodejs:master May 3, 2021
@Trott

Trott commented May 3, 2021

Copy link
Copy Markdown
Member Author

Landed in 18e4f40

@Trott
Trott deleted the pid-fix branch May 3, 2021 16:00
targos pushed a commit that referenced this pull request May 17, 2021
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 pushed a commit that referenced this pull request Jul 19, 2021
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

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
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 richardlau mentioned this pull request Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

7 participants