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.

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

test: remove duplicate required module #9169

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@Trott
Member

Trott commented Oct 18, 2016

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

test

Description of change

common is required twice. Remove second require() statement.

@Trott Trott added the test label Oct 18, 2016

@addaleax

LGTM

test/parallel/test-setproctitle.js Outdated
assert.equal(error, null);
assert.equal(stderr, '');
exec(`ps -p ${process.pid} -o args=`, function callback(error, stdout, stderr) {
assert.strictEqual(error, null);

This comment has been minimized.

@lpinca

lpinca Oct 18, 2016

Member

Nit: assert.ifError()?

@lpinca

lpinca approved these changes Oct 18, 2016

LGTM

@mscdex mscdex added the process label Oct 18, 2016

@Trott Trott force-pushed the Trott:duplicate branch to 6ddddb7 Oct 18, 2016

@Trott

This comment has been minimized.

Trott added a commit to Trott/io.js that referenced this pull request Oct 21, 2016

test: remove duplicate required module
`common` is required twice in test-setproctitle.js. Remove one of the
instances.

Other refactoring:

* var -> const and let
* assert.equal -> assert.strictEqual
* assert.notEqual -> assert.notStrickEqual
* string concatenation -> template string
* use of assert.ifError() instead of asserting error is null

PR-URL: nodejs#9169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott

This comment has been minimized.

Member

Trott commented Oct 21, 2016

Landed in 3184285

@Trott Trott closed this Oct 21, 2016

jasnell added a commit that referenced this pull request Oct 24, 2016

test: remove duplicate required module
`common` is required twice in test-setproctitle.js. Remove one of the
instances.

Other refactoring:

* var -> const and let
* assert.equal -> assert.strictEqual
* assert.notEqual -> assert.notStrickEqual
* string concatenation -> template string
* use of assert.ifError() instead of asserting error is null

PR-URL: #9169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Nov 15, 2016

test: remove duplicate required module
`common` is required twice in test-setproctitle.js. Remove one of the
instances.

Other refactoring:

* var -> const and let
* assert.equal -> assert.strictEqual
* assert.notEqual -> assert.notStrickEqual
* string concatenation -> template string
* use of assert.ifError() instead of asserting error is null

PR-URL: #9169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins added a commit that referenced this pull request Nov 15, 2016

test: remove duplicate required module
`common` is required twice in test-setproctitle.js. Remove one of the
instances.

Other refactoring:

* var -> const and let
* assert.equal -> assert.strictEqual
* assert.notEqual -> assert.notStrickEqual
* string concatenation -> template string
* use of assert.ifError() instead of asserting error is null

PR-URL: #9169
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

This was referenced Nov 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment