Skip to content

Conversation

@danbev
Copy link
Contributor

@danbev danbev commented Oct 13, 2017

This commit adds support to run the linter without running any other
targets. This commit also makes the lint task a little more quite and
more inline with the output on other operating systems.
Below is an example of running (with a lint error to show that it is not
so quite that errors are hidden):

C:\Users\danbev\working\node>vcbuild.bat lint
running lint-cpp 'src\*.c src\*.cc src\*.h test\addons\*.cc
test\addons\*.h test\cctest\*.cc test\cctest\*.h test\gc\binding.cc
tools\icu\*.cc tools\icu\*.h'
src\env.h:24:  Should have a space between // and comment
[whitespace/comments] [4]
Total errors found: 1
"C:\Python27\python.exe" tools/check-imports.py
running lint-js

The help message now looks like:

C:\Users\danbev\working\node>vcbuild.bat /?
vcbuild.bat [debug/release] [msi] [test/test-ci/test-all/test-uv/
test-inspector/test-internet/test-pummel/test-simple/test-message/
test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/
test-v8-all] [clean] [noprojgen] [small-icu/full-icu/without-intl]
[nobuild] [sign] [x86/x64] [vs2015/vs2017] [download-all]
[enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
[link-module path-to-module]
Examples:
  vcbuild.bat                          : builds release build
  vcbuild.bat debug                    : builds debug build
  vcbuild.bat release msi              : builds release build and MSI installer package
  vcbuild.bat test                     : builds debug build and runs tests
  vcbuild.bat build-release            : builds the release distribution as used by nodejs.org
  vcbuild.bat enable-vtune             : builds nodejs with Intel VTune profiling support to profile JavaScript
  vcbuild.bat link-module my_module.js : bundles my_module as built-in module
  vcbuild.bat lint                     : runs the C++ and JavaScript linter
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build, windows

This commit adds support to run the linter without running any other
targets. This commit also makes the lint task a little more quite and
more inline with the output on other operating systems.
Below is an example of running (with a lint error to show that it is not
so quite that errors are hidden):

C:\Users\danbev\working\node>vcbuild.bat lint
running lint-cpp 'src\*.c src\*.cc src\*.h test\addons\*.cc
test\addons\*.h test\cctest\*.cc test\cctest\*.h test\gc\binding.cc
tools\icu\*.cc tools\icu\*.h'
src\env.h:24:  Should have a space between // and comment
[whitespace/comments] [4]
Total errors found: 1
"C:\Python27\python.exe" tools/check-imports.py
running lint-js

The help message now looks like:
C:\Users\danbev\working\node>vcbuild.bat /?
vcbuild.bat [debug/release] [msi] [test/test-ci/test-all/test-uv/
test-inspector/test-internet/test-pummel/test-simple/test-message/
test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/
test-v8-all] [clean] [noprojgen] [small-icu/full-icu/without-intl]
[nobuild] [sign] [x86/x64] [vs2015/vs2017] [download-all]
[enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
[link-module path-to-module]
Examples:
  vcbuild.bat                          : builds release build
  vcbuild.bat debug                    : builds debug build
  vcbuild.bat release msi              : builds release build and MSI installer package
  vcbuild.bat test                     : builds debug build and runs tests
  vcbuild.bat build-release            : builds the release distribution as used by nodejs.org
  vcbuild.bat enable-vtune             : builds nodejs with Intel VTune profiling support to profile JavaScript
  vcbuild.bat link-module my_module.js : bundles my_module as built-in module
  vcbuild.bat lint                     : runs the C++ and JavaScript linter
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Oct 13, 2017
@vsemozhetbyt vsemozhetbyt added the tools Issues and PRs related to the tools directory. label Oct 13, 2017
@gireeshpunathil
Copy link
Member

@danbev
Copy link
Contributor Author

danbev commented Oct 16, 2017

test/windows-fanned unstable looks unrelated

console output:

not ok 470 inspector/test-bindings # TODO : Fix flaky test
  ---
  duration_ms: 0.250
  severity: flaky
  stack: |-
    Expecting warning to be emitted
    (node:6744) Error: We expect this
    c:\workspace\node-test-binary-windows\test\common\index.js:793
                 (err) => process.nextTick(() => { throw err; }));
                                                   ^
    
    AssertionError [ERR_ASSERTION]: [ 'Iteration 1 variable: i expected: 1 actual: 0',
      'Iteration 2 variable: i expected: 2 actual: 1',
      'Iteration 2 variable: a deepStrictEqual []
        at testSampleDebugSession (c:\workspace\node-test-binary-windows\test\inspector\test-bindings.js:104:10)
        at doTests (c:\workspace\node-test-binary-windows\test\inspector\test-bindings.js:131:3)
        at <anonymous>
        at process._tickCallback (internal/process/next_tick.js:188:7)
        at Function.Module.runMain (module.js:642:11)
        at startup (bootstrap_node.js:187:16)
        at bootstrap_node.js:605:3
  ...
not ok 471 inspector/test-debug-end # TODO : Fix flaky test
  ---
  duration_ms: 0.640
  severity: flaky
  stack: |-
    Test there's no crash stopping server that was not started
    Test there's no crash stopping server without connecting
    [err] Debugger listening on ws://127.0.0.1:63963/9a114a23-f790-487e-945f-176960414cdb
    [err] For help see https://nodejs.org/en/docs/inspector
    [err] 
    Test there's no crash stopping server after connecting
    [test] Connecting to a child Node process
    [test] Testing /json/list
    [err] Debugger listening on ws://127.0.0.1:63964/aa4f597e-619b-4d17-b144-e62d80daf6e0
    [err] For help see https://nodejs.org/en/docs/inspector
    [err] 
    [err] Debugger attached.
    [err] Debugger listening on ws://127.0.0.1:63967/70447c1e-6b5e-400a-858f-fca441a45ef0
    [err] For help see https://nodejs.org/en/docs/inspector
    [err] 
    { AssertionError [ERR_ASSERTION]: 42 === 3221225477
        at testSessionNoCrash (c:\workspace\node-test-binary-windows\test\inspector\test-debug-end.js:35:3)
        at <anonymous>
        at process._tickCallback (internal/process/next_tick.js:188:7)
      generatedMessage: true,
      name: 'AssertionError [ERR_ASSERTION]',
      code: 'ERR_ASSERTION',
      actual: 42,
      expected: 3221225477,
      operator: '===' }
    1

danbev added a commit to danbev/node that referenced this pull request Oct 17, 2017
This commit adds support to run the linter without running any other
targets. This commit also makes the lint task a little more quite and
more inline with the output on other operating systems.
Below is an example of running (with a lint error to show that it is not
so quite that errors are hidden):

C:\Users\danbev\working\node>vcbuild.bat lint
running lint-cpp 'src\*.c src\*.cc src\*.h test\addons\*.cc
test\addons\*.h test\cctest\*.cc test\cctest\*.h test\gc\binding.cc
tools\icu\*.cc tools\icu\*.h'
src\env.h:24:  Should have a space between // and comment
[whitespace/comments] [4]
Total errors found: 1
"C:\Python27\python.exe" tools/check-imports.py
running lint-js

The help message now looks like:
C:\Users\danbev\working\node>vcbuild.bat /?
vcbuild.bat [debug/release] [msi] [test/test-ci/test-all/test-uv/
test-inspector/test-internet/test-pummel/test-simple/test-message/
test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/
test-v8-all] [clean] [noprojgen] [small-icu/full-icu/without-intl]
[nobuild] [sign] [x86/x64] [vs2015/vs2017] [download-all]
[enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
[link-module path-to-module]
Examples:
  vcbuild.bat                          : builds release build
  vcbuild.bat debug                    : builds debug build
  vcbuild.bat release msi              : builds release build and MSI installer package
  vcbuild.bat test                     : builds debug build and runs tests
  vcbuild.bat build-release            : builds the release distribution as used by nodejs.org
  vcbuild.bat enable-vtune             : builds nodejs with Intel VTune profiling support to profile JavaScript
  vcbuild.bat link-module my_module.js : bundles my_module as built-in module
  vcbuild.bat lint                     : runs the C++ and JavaScript linter

PR-URL: nodejs#16176
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Oct 17, 2017

Landed in d78086b

@danbev danbev closed this Oct 17, 2017
@danbev danbev deleted the windows-linter branch October 17, 2017 05:45
targos pushed a commit that referenced this pull request Oct 18, 2017
This commit adds support to run the linter without running any other
targets. This commit also makes the lint task a little more quite and
more inline with the output on other operating systems.
Below is an example of running (with a lint error to show that it is not
so quite that errors are hidden):

C:\Users\danbev\working\node>vcbuild.bat lint
running lint-cpp 'src\*.c src\*.cc src\*.h test\addons\*.cc
test\addons\*.h test\cctest\*.cc test\cctest\*.h test\gc\binding.cc
tools\icu\*.cc tools\icu\*.h'
src\env.h:24:  Should have a space between // and comment
[whitespace/comments] [4]
Total errors found: 1
"C:\Python27\python.exe" tools/check-imports.py
running lint-js

The help message now looks like:
C:\Users\danbev\working\node>vcbuild.bat /?
vcbuild.bat [debug/release] [msi] [test/test-ci/test-all/test-uv/
test-inspector/test-internet/test-pummel/test-simple/test-message/
test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/
test-v8-all] [clean] [noprojgen] [small-icu/full-icu/without-intl]
[nobuild] [sign] [x86/x64] [vs2015/vs2017] [download-all]
[enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
[link-module path-to-module]
Examples:
  vcbuild.bat                          : builds release build
  vcbuild.bat debug                    : builds debug build
  vcbuild.bat release msi              : builds release build and MSI installer package
  vcbuild.bat test                     : builds debug build and runs tests
  vcbuild.bat build-release            : builds the release distribution as used by nodejs.org
  vcbuild.bat enable-vtune             : builds nodejs with Intel VTune profiling support to profile JavaScript
  vcbuild.bat link-module my_module.js : bundles my_module as built-in module
  vcbuild.bat lint                     : runs the C++ and JavaScript linter

PR-URL: #16176
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 18, 2017
This commit adds support to run the linter without running any other
targets. This commit also makes the lint task a little more quite and
more inline with the output on other operating systems.
Below is an example of running (with a lint error to show that it is not
so quite that errors are hidden):

C:\Users\danbev\working\node>vcbuild.bat lint
running lint-cpp 'src\*.c src\*.cc src\*.h test\addons\*.cc
test\addons\*.h test\cctest\*.cc test\cctest\*.h test\gc\binding.cc
tools\icu\*.cc tools\icu\*.h'
src\env.h:24:  Should have a space between // and comment
[whitespace/comments] [4]
Total errors found: 1
"C:\Python27\python.exe" tools/check-imports.py
running lint-js

The help message now looks like:
C:\Users\danbev\working\node>vcbuild.bat /?
vcbuild.bat [debug/release] [msi] [test/test-ci/test-all/test-uv/
test-inspector/test-internet/test-pummel/test-simple/test-message/
test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/
test-v8-all] [clean] [noprojgen] [small-icu/full-icu/without-intl]
[nobuild] [sign] [x86/x64] [vs2015/vs2017] [download-all]
[enable-vtune] [lint/lint-ci] [no-NODE-OPTIONS]
[link-module path-to-module]
Examples:
  vcbuild.bat                          : builds release build
  vcbuild.bat debug                    : builds debug build
  vcbuild.bat release msi              : builds release build and MSI installer package
  vcbuild.bat test                     : builds debug build and runs tests
  vcbuild.bat build-release            : builds the release distribution as used by nodejs.org
  vcbuild.bat enable-vtune             : builds nodejs with Intel VTune profiling support to profile JavaScript
  vcbuild.bat link-module my_module.js : bundles my_module as built-in module
  vcbuild.bat lint                     : runs the C++ and JavaScript linter

PR-URL: nodejs/node#16176
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

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

Labels

build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants