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.

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

v4.x backport - test: convert var->const/let in tests #11773

Closed
wants to merge 2 commits into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Mar 9, 2017

I'll fix up the commit messages once the tests are passing.

Backport of #10685, I couldn't cherry-pick as it brought in hundreds of unrelated changes, so I redid the eslint --fix and manual fixup for v4.x.

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

test

@gibfahn gibfahn added the wip Issues and PRs that are still a work in progress. label Mar 9, 2017
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. v4.x labels Mar 9, 2017
@gibfahn gibfahn force-pushed the var2const-v4.x-v2 branch 2 times, most recently from ddd770c to b7e4e34 Compare March 9, 2017 17:56
@gibfahn
Copy link
Member Author

gibfahn commented Mar 9, 2017

I'm getting an error for tests that have strict mode disabled, e.g. test-fs-stat#L40. I resolved it by disabling the no-var lint rule for those lines. If anyone objects please let me know.

=== release test-fs-stat ===                                                   
Path: parallel/test-fs-stat
/Users/gib/wrk/com/node/test/parallel/test-fs-stat.js:40
  let stats;
  ^^^

SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:373:25)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:140:18)
    at node.js:1043:3
Command: out/Release/node /Users/gib/wrk/com/node/test/parallel/test-fs-stat.js

Files failing:

  • test/parallel/test-fs-stat.js
  • test/parallel/test-module-nodemodulepaths.js
  • test/parallel/test-net-binary.js
  • test/parallel/test-repl.js
  • test/parallel/test-util-inspect.js

@gibfahn
Copy link
Member Author

gibfahn commented Mar 9, 2017

CI: https://ci.nodejs.org/job/node-test-commit/8345/

I'd appreciate people taking a look at the last three commits.

@gibfahn gibfahn force-pushed the var2const-v4.x-v2 branch 2 times, most recently from 803ae4a to dc72f29 Compare March 9, 2017 20:53
@MylesBorins
Copy link
Contributor

@gibfahn since v4.8.1 is likely going to be the last Active LTS release of v4.x and the first r.c. has already gone out it might make sense to skip this.

@gibfahn
Copy link
Member Author

gibfahn commented Mar 10, 2017

@MylesBorins obviously I'd like you to merge it if possible (having just finished wrestling it into shape), but it's up to you. The benefit to having this is that virtually nothing will backport cleanly to v4.x without it (same is true to a lesser extent of #10698 as well). Should also be low-risk as it's just testcase changes.

However I guess if we're really not expecting almost anything to be added to v4.x in the future I guess it might not be worth it, ¯\_(ツ)_/¯

EDIT: Whichever it is, let me know whether you want #10698 backported to v4.x, that one should be easier.

gibfahn and others added 2 commits March 10, 2017 00:50
Manually fix issues that eslint --fix couldn't do automatically.
@gibfahn
Copy link
Member Author

gibfahn commented Mar 10, 2017

I've rebased and rerun CI anyway, so @MylesBorins feel free to whatever you think is best!

CI 2: https://ci.nodejs.org/job/node-test-commit/8360/

The branch from before I squashed is var2const-v4.x-presquash, the squash! manual commits are the ones that might be worth reviewing.

@gibfahn gibfahn removed the wip Issues and PRs that are still a work in progress. label Mar 10, 2017
@gibfahn gibfahn changed the title WIP - v4.x backport - test: convert var->const/let in tests v4.x backport - test: convert var->const/let in tests Mar 10, 2017
@MylesBorins
Copy link
Contributor

I'm going to go ahead and close this for v4, will land the v6 version after this release

@gibfahn gibfahn deleted the var2const-v4.x-v2 branch March 11, 2017 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants