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.

Sign up for GitHub

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

Jump to bottom

Coverage reporter shallow error doc #52746

Closed
wants to merge 426 commits into from
Closed

Coverage reporter shallow error doc #52746

wants to merge 426 commits into from

Conversation

EliphazBouye
Copy link
Contributor

@EliphazBouye EliphazBouye commented Apr 29, 2024 •
edited
Loading

Add more information in the API test part doc about shallow error from coverage reporter #52670

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. test_runner Issues and PRs related to the test runner subsystem. labels Apr 29, 2024
@EliphazBouye EliphazBouye mentioned this pull request Apr 29, 2024
Closed
benjamingr
benjamingr reviewed Apr 29, 2024
View reviewed changes
doc/api/test.md Outdated
@@ -508,6 +508,9 @@ used as an in depth coverage report.
node --test --experimental-test-coverage --test-reporter=lcov --test-reporter-destination=lcov.info
```

* No test results are reported by this reporter.
* This reporter should ideally be used in conjunction with another reporter.
Copy link
Member

@benjamingr benjamingr Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This reporter should ideally be used in conjunction with another reporter.
* This reporter should ideally be used alongside another reporter.

"in conjunction" is probably harder to understand for ESL folk

atlowChemi
atlowChemi approved these changes Apr 29, 2024
View reviewed changes
Copy link
Member

@atlowChemi atlowChemi left a comment •
edited by MoLow
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, please rebase and edit the commit message of first commit to adhere to guidelines

@MoLow MoLow linked an issue May 15, 2024 that may be closed by this pull request
--test & --test-reporter & --test-reporter-destination will swallow error #52670
Closed
@H4ad
Copy link
Member

H4ad commented Jun 8, 2024 •
edited
Loading

@EliphazBouye Can you please update the first commit message to have less than 72 characters? (currently 73).

The suggestion by benjamin is also a good thing to change.

@EliphazBouye
Copy link
Contributor Author

Ok ok @H4ad I will do it now

gibbyfree and others added 18 commits June 20, 2024 21:48
@gibbyfree @EliphazBouye
test: use for-of instead of forEach
c300d67 
PR-URL: nodejs#49790
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruy Adorno <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@lemire @EliphazBouye
buffer: even faster atob
0a3cec9 
PR-URL: nodejs#52443
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@jcbhmr @EliphazBouye
bootstrap: print --help message using console.log
b78d767 
PR-URL: nodejs#51463
Fixes: nodejs#51448
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@kylo5aby @EliphazBouye
assert: add deep equal check for more Error type
a064469 
PR-URL: nodejs#51805
Fixes: nodejs#51793
Reviewed-By: Chengzhong Wu <[email protected]>
@xicilion @EliphazBouye
test, crypto: use correct object on assert
73d285c 
The test case for KeyObject does not really test the creation of
asymmetric cryptographic keys using jwk because of a misspelling of
the variable.

PR-URL: nodejs#51820
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@ulitink @EliphazBouye
test: add Debugger.setInstrumentationBreakpoint known issue
26bd936 
PR-URL: nodejs#31137
Refs: nodejs#31138
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@EliphazBouye
url,tools,benchmark: replace deprecated substr()
456b13c 
PR-URL: nodejs#51546
Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr
Reviewed-By: Jithil P Ponnan <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@daeyeon @EliphazBouye
src: fix Worker termination in inspector.waitForDebugger
ff11873 
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#52527
Fixes: nodejs#52467
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Kohei Ueno <[email protected]>
@Uzlopak @EliphazBouye
lib: add EventSource Client
805e7c5 
PR-URL: nodejs#51575
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@avivkeller @EliphazBouye
doc: update hljs with the latest styles
eca348b 
PR-URL: nodejs#52911
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
@huseyinacacak-janea @EliphazBouye
path: fix toNamespacedPath on Windows
d1a3fdd 
PR-URL: nodejs#52915
Fixes: nodejs#30224
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos @lemire
@StefanStojanovic @EliphazBouye
build: add option to enable clang-cl on Windows
d68b489 
Most changes are gated by the `clang==1` condition to avoid breaking
MSVC builds.

Select C/C++ language standard with ClCompile options.
This avoids passing the `-std:c++20` flag while compiling C code.
Do it only under clang option to avoid breaking addons until node-gyp
supports the new LanguageStandard options.

Disable precompiled header configuration for now as it doesn't seem to
work with clang-cl.

Disable C++20 warnings emitted by the Visual Studio C++ STL.
They're very noisy and not our responsibility to fix.

Co-authored-by: Daniel Lemire <[email protected]>
Co-authored-by: Stefan Stojanovic <[email protected]>
PR-URL: nodejs#52870
Reviewed-By: Daniel Lemire <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update corepack to 0.28.1
f2e200b 
PR-URL: nodejs#52946
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update simdjson to 3.9.2
fd0664d 
PR-URL: nodejs#52947
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update undici to 6.16.1
ea5d261 
PR-URL: nodejs#52948
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update googletest to fa6de7f
16d4b49 
PR-URL: nodejs#52949
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MoLow @EliphazBouye
test_runner: fix watch mode race condition
d14eeb4 
PR-URL: nodejs#52954
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@targos @EliphazBouye
tools: fix v8-update workflow
4598243 
- Add a step that configures Git so the update script can create
  commits.
- Use `peter-evans/create-pull-request` as it's more maintained and
  correctly handles commits that are created before it runs.

Refs: https://github.com/peter-evans/create-pull-request
PR-URL: nodejs#52957
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
nodejs-github-bot and others added 26 commits June 20, 2024 21:48
@nodejs-github-bot @EliphazBouye
deps: update simdjson to 3.9.4
c34e773 
PR-URL: nodejs#53467
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
@richardlau @EliphazBouye
src: fix dynamically linked OpenSSL version
4c6df95 
Report the version of OpenSSL that Node.js is running with instead
of the version of OpenSSL that Node.js was compiled against.

PR-URL: nodejs#53456
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@richardlau @EliphazBouye
test: check against run-time OpenSSL version
eb3aa37 
Update `common.hasOpenSSL3*` to check against the run-time version of
OpenSSL instead of the version of OpenSSL that Node.js was compiled
against.

Add a generalized `common.hasOpenSSL()` so we do not need to keep adding
new checks for each new major/minor of OpenSSL.

PR-URL: nodejs#53456
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@avivkeller @EliphazBouye
meta: fix typo in dependency updates
0be3549 
PR-URL: nodejs#53471
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
@anonrig @EliphazBouye
test: update url web-platform tests
38d843c 
PR-URL: nodejs#53472
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@joyeecheung @EliphazBouye
inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth
a37611c 
This was previously calling the enable function by mistake. As a
result, when profiling using Chrome DevTools, the async hooks won't
be turned off properly after receiving Debugger.setAsyncCallStackDepth
with depth 0.

PR-URL: nodejs#53473
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@targos @EliphazBouye
src: use args.This() instead of Holder
af84f2a 
The latter is deprecated in V8.

Refs: http://crbug.com/333672197
PR-URL: nodejs#53474
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
@anonrig @EliphazBouye
test: update encoding web-platform tests
eba2b53 
PR-URL: nodejs#53477
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@anonrig @EliphazBouye
test: update compression web-platform tests
1b4a91a 
PR-URL: nodejs#53478
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update undici to 6.19.0
2fa3ad9 
PR-URL: nodejs#53468
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update undici to 6.19.1
9b374f4 
PR-URL: nodejs#53468
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update undici to 6.19.1
4028f90 
PR-URL: nodejs#53468
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot @EliphazBouye
deps: update undici to 6.19.2
b12a3fe 
PR-URL: nodejs#53468
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
@nodejs-github-bot @EliphazBouye
meta: move one or more collaborators to emeritus
7a9e798 
PR-URL: nodejs#53480
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@kylo5aby @EliphazBouye
doc: update the description of the return type for options.filter
90ccef8 
PR-URL: nodejs#52742
Refs: nodejs#52461
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@kylo5aby @EliphazBouye
util: support --no- for argument with boolean type for parseArgs
5cfede6 
PR-URL: nodejs#53107
Refs: nodejs#53095
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@panva @EliphazBouye
doc: only apply content-visibility on all.html
414c598 
Refs: nodejs#40099
PR-URL: nodejs#53510
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Claudio Wunder <[email protected]>
@targos @EliphazBouye
tools: move ESLint to tools/eslint
ba99a3b 
Greatly simplify how ESLint and its plugins are installed.

PR-URL: nodejs#53413
Reviewed-By: Antoine du Hamel <[email protected]>
@anonrig @lemire @EliphazBouye
fs: move ToNamespacedPath to c++
74e5916 
Co-Authored-By: Daniel Lemire <[email protected]>
PR-URL: nodejs#52135
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Reviewed-By: Daniel Lemire <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jsk95 @EliphazBouye
stream: update outdated highwatermark doc
2e6905d 
PR-URL: nodejs#53494
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
@ehsankhfr @EliphazBouye
http2: reject failed http2.connect when used with promisify
888b966 
PR-URL: nodejs#53475
Reviewed-By: Tim Perry <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
@avivkeller @EliphazBouye
meta: use correct source for workflow in PR
7c7888c 
PR-URL: nodejs#53490
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@mhdawson @EliphazBouye
doc: add additional guidance for PRs to deps
aee48f6 
- add additional guidance based in discussion related
  to recent PR to dependency and discussion within the
  security-wg slack channel.

Refs: nodejs/security-wg#1329

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: nodejs#53499
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@marco-ippolito @EliphazBouye
2024-06-20, Version 20.15.0 'Iron' (LTS)
cc45998 
Notable changes:

doc:
  * add pimterry to collaborators (Tim Perry) nodejs#52874
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno) nodejs#52734
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig) nodejs#52860
tools:
  * (SEMVER-MINOR) fix get_asan_state() in tools/test.py (Joyee Cheung) nodejs#52766
  * (SEMVER-MINOR) support max_virtual_memory test configuration (Joyee Cheung) nodejs#52766
  * (SEMVER-MINOR) support != in test status files (Joyee Cheung) nodejs#52766
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung) nodejs#52692

PR-URL: nodejs#53486
@richardlau @EliphazBouye
src: reset process.versions during pre-execution
64fe5ff 
Reset `process.versions` during pre-execution so that it reflects
the versions at run-time rather than when the snapshot was taken.

PR-URL: nodejs#53444
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@EliphazBouye
doc: change "in conjunction with" to "alongside"↲
e0e942c 
Changed for better understanding↲
@EliphazBouye EliphazBouye force-pushed the coverage-reporter-shallow-error-doc branch from 8bd97f4 to e0e942c Compare June 20, 2024 21:49
@EliphazBouye
Copy link
Contributor Author

Is it good now? @H4ad sorry for the force-pushed message. I try to undo and try again with simple push but it does not work. do you have a solution to solve it ? or it does not cause problem for this particular case ?

@EliphazBouye EliphazBouye deleted the coverage-reporter-shallow-error-doc branch June 20, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingr benjamingr benjamingr approved these changes

@MoLow MoLow MoLow approved these changes

@himself65 himself65 himself65 approved these changes

@atlowChemi atlowChemi atlowChemi approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

--test & --test-reporter & --test-reporter-destination will swallow error