Skip to content

Conversation

@Trott
Copy link
Member

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

@github-actionsgithub-actionsbot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 27, 2021
@Trott
Copy link
MemberAuthor

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

@TrottTrott 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
Collaborator

//
// .then(() => cli.waitForPrompt())
//
// What we're diong for now:
Copy link
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
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
Copy link
MemberAuthor

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
Copy link
MemberAuthor

OK, I've generalized the workaround. PTAL

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@TrottTrott changed the title test: fix flaky inspector-cli/test-inspector-cli-pid on AIXtest: fix flaky inspector-cli tests when breakpoints are restoredMay 3, 2021
PR-URL: nodejs#38431 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
@TrottTrott merged commit 18e4f40 into nodejs:masterMay 3, 2021
@Trott
Copy link
MemberAuthor

Trott commented May 3, 2021

Landed in 18e4f40

@TrottTrott 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 <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
@targostargos mentioned this pull request May 18, 2021
targos pushed a commit to Trott/io.js that referenced this pull request Jun 6, 2021
PR-URL: nodejs#38431 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
targos pushed a commit to Trott/io.js that referenced this pull request Jun 6, 2021
PR-URL: nodejs#38431 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
richardlau pushed a commit that referenced this pull request Jul 19, 2021
PR-URL: #38431 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
@richardlau
Copy link
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 <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
@richardlaurichardlau mentioned this pull request Jul 20, 2021
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38431 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aixIssues and PRs related to the AIX platform.flaky-testIssues and PRs related to the tests with unstable failures on the CI.inspectorIssues and PRs related to the V8 inspector protocolneeds-ciPRs that need a full CI run.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott@nodejs-github-bot@richardlau@jasnell@gireeshpunathil@RaisinTen@targos