test: modernize JS and tighten equality checking #8476

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
8 participants
@pogilvie
Contributor

pogilvie commented Sep 10, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

Node todo process example with the following test-net-binary.js changes:

var --> const where applicable

==, assert.equal--> ===, assert.strictEqual for all cases

test: modernize JS and tighten equality checking
Node todo process example with the follow test-net-binary.js changes:

var --> const where applicable

==, assert.equal--> ===, assert.strictEqual for all cases
test/parallel/test-net-binary.js
- var S = eval(s);
- assert.ok(S.charCodeAt(0) == i);
- assert.ok(S == String.fromCharCode(i));
+ const s = '\'\\' + i.toString(8) + '\'';

This comment has been minimized.

@cjihrig

cjihrig Sep 10, 2016

Contributor

This could probably be made more readable using a template literal.

@cjihrig

cjihrig Sep 10, 2016

Contributor

This could probably be made more readable using a template literal.

@cjihrig

This comment has been minimized.

Show comment
Hide comment
@cjihrig

cjihrig Sep 10, 2016

Contributor

LGTM with a comment.

Contributor

cjihrig commented Sep 10, 2016

LGTM with a comment.

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Sep 10, 2016

Member

Pending CI, LGTM with @cjihrig's comment.

Member

Trott commented Sep 10, 2016

Pending CI, LGTM with @cjihrig's comment.

@thefourtheye

This comment has been minimized.

Show comment
Hide comment
@thefourtheye

thefourtheye Sep 10, 2016

Contributor

LGTM, with @cjihrig's comment.

Contributor

thefourtheye commented Sep 10, 2016

LGTM, with @cjihrig's comment.

@Trott

This comment has been minimized.

Show comment
Hide comment
@pogilvie

This comment has been minimized.

Show comment
Hide comment
@pogilvie

pogilvie Sep 10, 2016

Contributor

Please see suggested update. Is this what you're suggesting? thx.

Contributor

pogilvie commented Sep 10, 2016

Please see suggested update. Is this what you're suggesting? thx.

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Sep 12, 2016

Member

LGTM

Member

jasnell commented Sep 12, 2016

LGTM

@lpinca

This comment has been minimized.

Show comment
Hide comment
@lpinca

lpinca Sep 12, 2016

Member

LGTM

Member

lpinca commented Sep 12, 2016

LGTM

Trott added a commit that referenced this pull request Sep 13, 2016

test: modernize JS and tighten equality checking
Node todo process example with the follow test-net-binary.js changes:

var --> const where applicable

==, assert.equal--> ===, assert.strictEqual for all cases

PR-URL: #8476
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Sep 13, 2016

Member

Landed in 759cf17

Member

Trott commented Sep 13, 2016

Landed in 759cf17

@Trott Trott closed this Sep 13, 2016

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

test: modernize JS and tighten equality checking
Node todo process example with the follow test-net-binary.js changes:

var --> const where applicable

==, assert.equal--> ===, assert.strictEqual for all cases

PR-URL: #8476
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Sep 30, 2016

Member

this is oddly causing failures when backported... seems to be the strict equality. Perhaps there is a bug on v4.x

Member

MylesBorins commented Sep 30, 2016

this is oddly causing failures when backported... seems to be the strict equality. Perhaps there is a bug on v4.x

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