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

Conversation

@hybrist
Copy link
Collaborator

@hybrist hybrist commented Mar 15, 2017

The port export no longer exists and isn't in use. It was a hold-over from the old built-in debugger.

package.json Outdated
"scripts": {
"pretest": "eslint --rulesdir=tools/eslint-rules lib test",
"test": "tap \"test/**/*.test.js\"",
"test": "tap \"test/cli/*.test.js\"",
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 glob is interpreted differently, depending on who is running our test suite. This means that if we ever add tests outside of test/cli we'll have to change this.

@hybrist
Copy link
Collaborator Author

hybrist commented Mar 15, 2017

/cc @ofrobots Missed this one in the initial PR (because of the broken glob).

@hybrist hybrist mentioned this pull request Mar 28, 2017
@hybrist hybrist force-pushed the jk-embedded-failures branch from ef6a485 to 41148d7 Compare April 3, 2017 19:39
@hybrist
Copy link
Collaborator Author

hybrist commented Apr 3, 2017

@hybrist
Copy link
Collaborator Author

hybrist commented Apr 3, 2017

Woah, guess a lot changed in master recently.

@hybrist hybrist changed the title Remove outdated test Update test suite to pass on latest nightly Apr 3, 2017
@hybrist
Copy link
Collaborator Author

hybrist commented Apr 3, 2017

The new Break on start feature works great - but it stops at the function wrapper, not on the first line of the function wrapper. So everywhere we start or restart, we need to skip it.

@hybrist
Copy link
Collaborator Author

hybrist commented Apr 3, 2017

Passing CI against latest nightly: https://ci.nodejs.org/job/node-inspect-continuous-integration/28/

Copy link

@dbushong dbushong left a comment

Choose a reason for hiding this comment

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

LGTM

"scripts": {
"pretest": "eslint --rulesdir=tools/eslint-rules lib test",
"test": "tap \"test/**/*.test.js\"",
"test": "tap test",
Copy link

Choose a reason for hiding this comment

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

Is it ok that this might load non-test js files? (ones intended to be require()ed from elsewhere?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now there's only one of those (test/cli/start-cli.js) which is a bit annoying. In future, I'd rather move those helpers into a sibling directory (test-helpers/) than risk the inconsistent glob behavior.

@hybrist hybrist merged commit 7b31acf into master Apr 3, 2017
@hybrist hybrist deleted the jk-embedded-failures branch April 3, 2017 21:29
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.

3 participants