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: remove require('buffer') on 4 test files #13844

Closed
wants to merge 1 commit into from
Closed

test: remove require('buffer') on 4 test files #13844

wants to merge 1 commit into from

Conversation

XadillaX
Copy link
Contributor

@XadillaX XadillaX commented Jun 21, 2017 •
edited
Loading

We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

  • test/parallel/test-stream-uint8array.js
  • test/parallel/test-stream2-finish-pipe.js
  • test/parallel/test-tls-session-cache.js
  • test/parallel/test-vm-cached-data.js

Refs: #13836

Checklist
  • make -j4 test (UNIX)
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test, buffer

@XadillaX
test: remove require('buffer') on 4 test files
68acc80 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: nodejs#13836
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 21, 2017
@vsemozhetbyt
Copy link
Contributor

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

@refack refack mentioned this pull request Jun 21, 2017
Closed
@mscdex mscdex added stream Issues and PRs related to the stream subsystem. tls Issues and PRs related to the tls subsystem. vm Issues and PRs related to the vm subsystem. labels Jun 21, 2017
benjamingr
benjamingr approved these changes Jun 21, 2017
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Nice work. Please feel free to also give us any feedback about the onboarding process today.

#goodnessSquad

@benjamingr benjamingr mentioned this pull request Jun 22, 2017
Closed
aqrln pushed a commit that referenced this pull request Jun 24, 2017
@XadillaX @aqrln
test: remove require('buffer') from 4 test files
befede6 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@aqrln
Copy link
Contributor

aqrln commented Jun 24, 2017

Landed in befede6. Thanks a lot for making an example for the first-time contributors, I see that some pull requests just followed your template :)

@aqrln aqrln closed this Jun 24, 2017
addaleax pushed a commit that referenced this pull request Jun 24, 2017
@XadillaX @addaleax
test: remove require('buffer') from 4 test files
89586a8 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@addaleax addaleax mentioned this pull request Jun 24, 2017
Merged
addaleax pushed a commit that referenced this pull request Jun 29, 2017
@XadillaX @addaleax
test: remove require('buffer') from 4 test files
339a7d3 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@XadillaX @addaleax
test: remove require('buffer') from 4 test files
b504d3d 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@XadillaX @addaleax
test: remove require('buffer') from 4 test files
005e343 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 14, 2017
@XadillaX @MylesBorins
test: remove require('buffer') from 4 test files
47dd8f0 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@XadillaX @MylesBorins
test: remove require('buffer') from 4 test files
ea2a7f8 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@XadillaX @MylesBorins
test: remove require('buffer') from 4 test files
8ba7843 
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:

+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js

Refs: #13836
PR-URL: #13844
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
Merged
This was referenced Sep 5, 2017
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

@mhdawson mhdawson mhdawson approved these changes

@aqrln aqrln aqrln approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@lpinca lpinca lpinca approved these changes

@benjamingr benjamingr benjamingr approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem. 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.