From ed4934dff3f8fad085fc60e8d5b5b10b53ea4628 Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Sun, 28 Oct 2018 22:59:56 -0700 Subject: [PATCH 1/3] doc: moved test instructions to BUILDING.md Fixes: https://github.com/nodejs/node/issues/23491 Duplicate test instructions were present in pull-requests.md Merged the instructions in BUILDING.md and provided a link from pull-requests.md --- BUILDING.md | 35 +++++++++-- doc/guides/contributing/pull-requests.md | 74 +----------------------- 2 files changed, 33 insertions(+), 76 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 81157a91a615fd..938de5f19a7179 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -24,6 +24,7 @@ file a new issue. * [Prerequisites](#prerequisites) * [Building Node.js](#building-nodejs-1) * [Running Tests](#running-tests) + * [Running Coverage](#running-coverage) * [Building the documentation](#building-the-documentation) * [Building a debug build](#building-a-debug-build) * [Windows](#windows-1) @@ -223,6 +224,13 @@ $ make -j4 test `make -j4 test` does a full check on the codebase, including running linters and documentation tests. +Make sure the linter does not report any issues and that all tests pass. Please +do not submit patches that fail either check. + +If you want to run the linter without running tests, use +`make lint`/`vcbuild lint`. It will run both JavaScript linting and +C++ linting. + If you are updating tests and just want to run a single test to check it: ```text @@ -249,18 +257,35 @@ You can usually run tests directly with node: $ ./node ./test/parallel/test-stream2-transform.js ``` -Optionally, continue below. +Remember to recompile with `make -j4` in between test runs if you change code in +the `lib` or `src` directories. + +#### Running Coverage -To run the tests and generate code coverage reports: +It's good practice to ensure any code you add or change is covered by tests. +You can do so by running the test suite with coverage enabled: ```console $ ./configure --coverage $ make coverage ``` -This will generate coverage reports for both JavaScript and C++ tests (if you -only want to run the JavaScript tests then you do not need to run the first -command `./configure --coverage`). +A detailed coverage report will be written to `coverage/index.html` for +JavaScript coverage and to `coverage/cxxcoverage.html` for C++ coverage. +(if you only want to run the JavaScript tests then you do not need to run +the first command `./configure --coverage`). + +_Note that generating a test coverage report can take several minutes._ + +To collect coverage for a subset of tests you can set the `CI_JS_SUITES` and +`CI_NATIVE_SUITES` variables: + +```text +$ CI_JS_SUITES=child-process CI_NATIVE_SUITES= make coverage +``` + +The above command executes tests for the `child-process` subsystem and +outputs the resulting coverage report. The `make coverage` command downloads some tools to the project root directory and overwrites the `lib/` directory. To clean up after generating the coverage diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index b83624ac5febbb..15c31cec714614 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -15,7 +15,6 @@ so that you can make the actual changes. This is where we will start. * [Commit message guidelines](#commit-message-guidelines) * [Step 5: Rebase](#step-5-rebase) * [Step 6: Test](#step-6-test) - * [Test Coverage](#test-coverage) * [Step 7: Push](#step-7-push) * [Step 8: Opening the Pull Request](#step-8-opening-the-pull-request) * [Step 9: Discuss and Update](#step-9-discuss-and-update) @@ -237,75 +236,7 @@ And on Windows: > vcbuild test ``` -(See the [Building guide][] for more details.) - -Make sure the linter does not report any issues and that all tests pass. Please -do not submit patches that fail either check. - -If you want to run the linter without running tests, use -`make lint`/`vcbuild lint`. It will run both JavaScript linting and -C++ linting. - -If you are updating tests and just want to run a single test to check it: - -```text -$ python tools/test.py -J --mode=release parallel/test-stream2-transform -``` - -You can execute the entire suite of tests for a given subsystem -by providing the name of a subsystem: - -```text -$ python tools/test.py -J --mode=release child-process -``` - -If you want to check the other options, please refer to the help by using -the `--help` option - -```text -$ python tools/test.py --help -``` - -You can usually run tests directly with node: - -```text -$ ./node ./test/parallel/test-stream2-transform.js -``` - -Remember to recompile with `make -j4` in between test runs if you change code in -the `lib` or `src` directories. - -#### Test Coverage - -It's good practice to ensure any code you add or change is covered by tests. -You can do so by running the test suite with coverage enabled: - -```text -$ ./configure --coverage && make coverage -``` - -A detailed coverage report will be written to `coverage/index.html` for -JavaScript coverage and to `coverage/cxxcoverage.html` for C++ coverage. - -_Note that generating a test coverage report can take several minutes._ - -To collect coverage for a subset of tests you can set the `CI_JS_SUITES` and -`CI_NATIVE_SUITES` variables: - -```text -$ CI_JS_SUITES=child-process CI_NATIVE_SUITES= make coverage -``` - -The above command executes tests for the `child-process` subsystem and -outputs the resulting coverage report. - -Running tests with coverage will create and modify several directories -and files. To clean up afterwards, run: - -```text -make coverage-clean -./configure && make -j4. -``` +(See the [running tests][] section of Building guide for more details.) ### Step 7: Push @@ -661,7 +592,8 @@ If you want to know more about the code review and the landing process, see the [Code of Conduct]: https://github.com/nodejs/admin/blob/master/CODE_OF_CONDUCT.md [Collaborator Guide]: ../../../COLLABORATOR_GUIDE.md [guide for writing tests in Node.js]: ../writing-tests.md +[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment [https://ci.nodejs.org/]: https://ci.nodejs.org/ [IRC in the #node-dev channel]: https://webchat.freenode.net?channels=node-dev&uio=d4 [Onboarding guide]: ../../onboarding.md -[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment +[running tests]: ../../../BUILDING.md#running-tests \ No newline at end of file From 6edeb2a990563e61efe4e16e1ca2840a9237ffb6 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Mon, 29 Oct 2018 17:13:59 -0700 Subject: [PATCH 2/3] f Suggestion from @vsemozhetbyt Co-Authored-By: trivikr <16024985+trivikr@users.noreply.github.com> --- BUILDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILDING.md b/BUILDING.md index 938de5f19a7179..99d9d51777ae0e 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -271,7 +271,7 @@ $ make coverage ``` A detailed coverage report will be written to `coverage/index.html` for -JavaScript coverage and to `coverage/cxxcoverage.html` for C++ coverage. +JavaScript coverage and to `coverage/cxxcoverage.html` for C++ coverage (if you only want to run the JavaScript tests then you do not need to run the first command `./configure --coverage`). From 2bdbde8a29b54f00efb1093a4dd4280ada6a5cd3 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Mon, 29 Oct 2018 17:26:41 -0700 Subject: [PATCH 3/3] f applying suggestion from @trott --- BUILDING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILDING.md b/BUILDING.md index 99d9d51777ae0e..36dd2dd3de6f55 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -275,7 +275,7 @@ JavaScript coverage and to `coverage/cxxcoverage.html` for C++ coverage (if you only want to run the JavaScript tests then you do not need to run the first command `./configure --coverage`). -_Note that generating a test coverage report can take several minutes._ +_Generating a test coverage report can take several minutes._ To collect coverage for a subset of tests you can set the `CI_JS_SUITES` and `CI_NATIVE_SUITES` variables: