Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Delay run until breakpoints are restored #34

Merged
merged 5 commits into from
Mar 15, 2017
Merged

Delay run until breakpoints are restored #34

merged 5 commits into from
Mar 15, 2017

Conversation

jkrems
Copy link
Collaborator

@jkrems jkrems commented Mar 14, 2017

Tested this by delaying the ready event by 250ms and going from there.

@@ -8,7 +8,9 @@ const startCLI = require('./start-cli');
test('examples/empty.js', (t) => {
const script = Path.join('examples', 'empty.js');
const cli = startCLI([script]);
return cli.waitForPrompt()

return cli.waitFor(/break/)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This matches how all other test files are doing it. There really should be a cli.waitForInitialPrompt helper.

@jkrems
Copy link
Collaborator Author

jkrems commented Mar 14, 2017

@jkrems
Copy link
Collaborator Author

jkrems commented Mar 14, 2017

Down to one win10 failure ("sb before loading file"): https://ci.nodejs.org/view/x%20-%20Diagnostics/job/node-inspect-continuous-integration/17/MACHINE=win10/console

EDIT: Nevermind, that's actually an 8.x-nightly failure in general. Can reproduce on OSX.

EDIT 2: The same failure ("sb before loading file") also reproduces on latest 7.x.


const startCLI = require('./start-cli');

test('for whiles that starts with strict directive', (t) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added this test for documentation purposes and to make sure that adding a strict directive doesn't actually break everything.

@jkrems
Copy link
Collaborator Author

jkrems commented Mar 15, 2017

/cc @nodejs/diagnostics

Now the build is all green for node 6 (smartos fails way before it starts running anything): https://ci.nodejs.org/job/node-inspect-continuous-integration/22/

Build against 8.x-nightly 100% green: https://ci.nodejs.org/job/node-inspect-continuous-integration/23/

P.S.: Yes, for AIX I'm cheating a bit. Still worth investigating but it's at least highly localized and mostly cosmetic.

@jkrems jkrems mentioned this pull request Mar 15, 2017
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM. Since the AIX issue is different from #33 I don't think we have an open issue covering the specific failure the aix check works around. Can you open an issue that shows the failure in that case ?

@jkrems jkrems merged commit 6052946 into master Mar 15, 2017
@jkrems jkrems deleted the jk-delay-run branch March 15, 2017 16:09
@jkrems
Copy link
Collaborator Author

jkrems commented Mar 15, 2017

Created #35 for following up on the AIX issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants