Skip to content

Conversation

@burgerboydaddy
Copy link
Contributor

Added additional information to assert error messages to indicate possible problem inside test/parallel/test-vm-context.js

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

PR
test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdex mscdex added the vm Issues and PRs related to the vm subsystem. label Oct 6, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 7, 2017
@addaleax
Copy link
Member

addaleax commented Oct 7, 2017

// Issue GH-1140:
// Test runInContext signature
let gh1140Exception;
let gh1140Exp;
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the value in changing the variable name (no idea what Exp is supposed to mean).

Copy link
Member

Choose a reason for hiding this comment

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

+1 ... generally we use more expressive variable names as much as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proper fix is implemented in pull request #16116

@jasnell
Copy link
Member

jasnell commented Oct 10, 2017

Closing in favor of the new pr.

@jasnell jasnell closed this Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants