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

Update test suite to pass on latest nightly #36

Merged
merged 4 commits into from
Apr 3, 2017
Merged

Conversation

jkrems
Copy link
Collaborator

@jkrems jkrems 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
@@ -15,7 +15,7 @@
},
"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.

@jkrems
Copy link
Collaborator Author

jkrems commented Mar 15, 2017

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

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

jkrems commented Apr 3, 2017

@jkrems
Copy link
Collaborator Author

jkrems commented Apr 3, 2017

Woah, guess a lot changed in master recently.

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

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

@jkrems
Copy link
Collaborator Author

jkrems 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

@@ -15,7 +15,7 @@
},
"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.

@jkrems jkrems merged commit 7b31acf into master Apr 3, 2017
@jkrems jkrems 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.

2 participants