Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: improve test-vm-cached-data.js #11974

Closed
wants to merge 1 commit into from
Closed

test: improve test-vm-cached-data.js #11974

wants to merge 1 commit into from

Conversation

mpelekh
Copy link
Contributor

@mpelekh mpelekh commented Mar 21, 2017 •
edited
Loading

  • verify error message by adding 2nd argument to throws in test-assert
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 21, 2017
@mscdex mscdex added the vm Issues and PRs related to the vm subsystem. label Mar 21, 2017
cjihrig
cjihrig approved these changes Mar 21, 2017
View reviewed changes
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM with a nit.

test/parallel/test-vm-cached-data.js Outdated
@@ -84,9 +84,12 @@ function testRejectSlice() {
}
testRejectSlice();

const expectedError =
/^TypeError: options.cachedData must be a Buffer instance$/;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is only used in one place, it probably doesn't need to go in a variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right! I'll change it.

@mpelekh
test: improve test-vm-cached-data.js
77d7057 
* verify error message by adding 2nd argument to throws in test-assert
@lpinca
Copy link
Member

lpinca commented Mar 23, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/7000/

jasnell pushed a commit that referenced this pull request Mar 24, 2017
@mpelekh @jasnell
test: improve test-vm-cached-data.js
89d9c3f 
* verify error message by adding 2nd argument to throws in test-assert

PR-URL: #11974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@jasnell
Copy link
Member

jasnell commented Mar 24, 2017

Landed in 89d9c3f

@jasnell jasnell closed this Mar 24, 2017
MylesBorins pushed a commit that referenced this pull request Mar 28, 2017
@mpelekh @MylesBorins
test: improve test-vm-cached-data.js
e70ed3c 
* verify error message by adding 2nd argument to throws in test-assert

PR-URL: #11974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 28, 2017
Merged
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
Merged
2 tasks
MylesBorins pushed a commit that referenced this pull request Apr 18, 2017
@mpelekh @MylesBorins
test: improve test-vm-cached-data.js
f3f4ac4 
* verify error message by adding 2nd argument to throws in test-assert

PR-URL: #11974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
@mpelekh @MylesBorins
test: improve test-vm-cached-data.js
6063a4a 
* verify error message by adding 2nd argument to throws in test-assert

PR-URL: #11974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@mpelekh @andrew749
test: improve test-vm-cached-data.js
9440467 
* verify error message by adding 2nd argument to throws in test-assert

PR-URL: nodejs/node#11974
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@watilde watilde watilde approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mpelekh @lpinca @jasnell @watilde @cjihrig @mscdex @MylesBorins @nodejs-github-bot