Enhancing tests of config parse - Rebuilt #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feat: Adding additional configuration file options test: unit test for config file detection test: unit test for config files pass via the cli test: unit test case for early exit of hideInstrumenteeArgs test: unit test for Node.js argument handling fix: bug in spawning of c8 process that dropped coverage significantly
Logically these are not part of the argument parsing. They are configuration loading and validation. Having them in the parse-args module was only making the file and the tests harder to work with. This change only has two semantic differences: 1. The list of known config files names was converted to a const immutable array instead of beating a mutable array returned by a function. 2. The parse-args file is no longer exporting those two internal utility functions, instead the equivalents are coming from a dedicated file.
Specifically: - Handling the cases of empty files and files that have simple forms of improper content. - Reporting easier to understand errors when the parsers fail. Adding schema validators was deemed beyond the scope of this particular commit, though notes were added for where they should be added. Incorporated many of mcknasty's suggestions. However I didn't keep them all, and many other elements were reinterpreted.
|
Locally $ npm -v; node -v
6.14.18
v14.21.3
$ npm ci
npm WARN prepare removing existing node_modules/ before installation
added 384 packages in 2.167s
$ npm -v; node -v
8.19.4
v16.20.2
$ npm ci
added 384 packages, and audited 385 packages in 3s
98 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
$ npm -v; node -v
10.2.3
v18.19.0
$ npm ci
added 384 packages, and audited 385 packages in 2s
98 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
$ npm -v; node -v
10.2.4
v20.11.0
$ npm ci
added 384 packages, and audited 385 packages in 2s
98 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities |
|
This is looking really good. This is based on the branch I created and this pull request. It should have all your changes from this morning. I need to clean up the commit log a bit more. I was going to add my name to your last commit, if you don't mind, due to some of the work on the test cases, being the original branch owner. |
mcknasty
force-pushed
the
additional-config-files
branch
2 times, most recently
from
fd3e9d4
to
6ff250f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an upgraded version of #5
I incorporated a lot of @mcknasty's suggestions and even went further by enhancing the error API and testing it, plus several other minor cleanups, comments, and JSDoc additions.