From 3e2dc46b35f8d8f238c2b10cff7f617736384887 Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Sun, 22 Oct 2023 13:31:28 +0200 Subject: [PATCH 1/3] Improve the Contributing Guidelines (#1252) Did a pass through the Contributing Guidelines in order to find typos, outdated information, poorly worded/explained concepts, or duplicated copy - and fix them. --- CONTRIBUTING.md | 150 ++++++++++++++++++++++++++---------------------- 1 file changed, 82 insertions(+), 68 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0ad7924c2..39a8836e0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ When you have a clear idea of what you need, you can submit a [feature request]. ### Corrections Corrections, such as fixing typos or refactoring code, are important. For small -changes you can open a Pull Request directly, Or you can first [open an issue]. +changes you can open a Pull Request directly, or you can first [open an issue]. --- @@ -162,26 +162,24 @@ the benchmarks locally using `npm run benchmark`. #### Typings Even though this project is written in JavaScript, it provides [TypeScript] type -definitions out-of-the-box. All type definitions are specified in `index.d.ts`. -This file only needs to change if the public API of the project changes. +definitions out-of-the-box. All type definitions are specified in `*.d.ts`. Such +files only need to change if the public API of the project changes. #### Building -The source code is transpiled and bundled into a CommonJS files at `./*.cjs` -with [rollup.js] when the package is published to npm. This is to provide -support for older Node.js versions. Run `npm run transpile` locally to create -these files. Note that these files are ignored by git. +The source code is transpiled and bundled into a CommonJS files, `*.cjs` and +`.d.cts`, with [rollup.js] when the package is published to npm. This is done to +provide support for older Node.js versions. Run `npm run transpile` locally to +create these files. Note that these files are ignored by git. #### Auditing ##### Licenses -To audit the licenses of npm dependencies, use `npm run license-check`. If no -problems are detected this will output nothing, else a list of packages without -approved licenses is outputted. - -The license check runs [licensee] to validate that the licenses of dependencies -are allowed (or have been manually reviewed in the past). +To audit the licenses of npm dependencies, use `npm run license-check`. This +uses runs [licensee] to validate that the licenses of dependencies are allowed +or have been manually reviewed in the past. If no problems are detected this +will output nothing, else a list of packages with unapproved licenses is shown. ##### Vulnerabilities @@ -218,6 +216,7 @@ To run tests use `npm run [SCRIPT]:[MODIFIER]`, e.g. `npm run test:unit` or | `test`, `coverage` | `e2e` | Run end-to-end (e2e) tests | | `test`, `coverage` | `compat` | Run compatibility tests on current Node.js version | | `test` | `compat-all` | Run compatibility tests on supported Node.js versions | +| `test`, `coverage` | `breakage` | Run breakage tests | | `fuzz` | _None_ | Run fuzz tests | Whenever you use the `coverage` variant of a script, a code coverage report will @@ -227,8 +226,8 @@ variant of a script, a mutant report will be generated at `_reports/mutation/`. ### Test Organization The tests for the project are organized into the levels unit, integration, -end-to-end (e2e), and compatibility. Each level focusses on testing different -aspects of the project. +end-to-end (e2e), compatibility, and breakage. Each level focusses on testing +different aspects of the project. #### Unit Testing @@ -241,20 +240,11 @@ folder. Each file in `src/` is represented by a folder in the test structure, where files represent individual units within the respective file in `src/`. When writing unit tests, aim to test one thing at the time. Correspondingly, the -test title should describe what is being test - not how it is tested, or what is -expected. - -##### Mutation Testing Unit Tests +test title should describe what is being tested - not how it is tested, or what +is expected. -The effectiveness of the unit tests is ensured by [mutation testing] (using -[Stryker]). You can run mutation tests for unit tests using the command -`npm run mutation:unit`, which will generate a mutation report at -`_reports/mutation/unit.html`. - -After you make changes to the `src/` code and have added tests, consider running -mutation tests. Running mutation tests will tell you if there are behaviour -changing modification that can be made to the source without the unit tests -catching the change. Such modifications are labeled as _Survived_. +The effectiveness of the unit tests is ensured through [mutation testing]. You +can run mutation tests for unit tests using the command `npm run mutation:unit`. #### Integration Testing @@ -269,17 +259,9 @@ API. When writing integration tests, focus on behavior that emerges from the composition of units. -##### Mutation Testing Integration Tests - -Like unit tests, the effectiveness of the integration tests is ensured by -[mutation testing] (using [Stryker]). You can run mutation tests for integration -tests using the command `npm run mutation:integration`, which will generate a -mutation report at `_reports/mutation/integration.html`. - -After you make changes to `index.js` and have added tests, consider running -mutation tests. Running mutation tests will tell you if there are behaviour -changing modification that can be made to `index.js` without the integration -tests catching the change. Such modifications are labeled as _Survived_. +The effectiveness of the integration tests is ensured by [mutation testing]. You +can run mutation tests for integration tests using the command +`npm run mutation:integration`. #### End-to-end Testing @@ -295,15 +277,15 @@ the shell. ##### End-to-end Fuzz Testing -Additionally, there are also end-to-end [fuzz tests] (using [Jsfuzz]) for this -project. All fuzz tests go into the `test/fuzz/` folder. You can start fuzzing -using the command `npm run fuzz`, which will provide more instructions. +There are also end-to-end [fuzz tests] (using [Jsfuzz]) for this project. All +fuzz tests go into the `test/fuzz/` folder. You can start fuzzing using the +command `npm run fuzz`, which will provide more instructions. -Fuzz tests aim to find logic flaws or unhandled error scenarios. If you improve -or add to the fuzz code, please share your improvements. Note that fuzz logic -must be written in CommonJS (a requirement from [Jsfuzz]). Due to the use of -CommonJS for fuzz code and ES Modules for source code, the coverage report from -Jsfuzz is empty and not used (coverage guided fuzzing is also not available). +When writing fuzz tests the goal is to find unknown bugs, logic flaws, and +unhandled error scenarios. Note that fuzz logic must be written in CommonJS (a +requirement from [Jsfuzz]). Due to the use of CommonJS for fuzz code and ES +Modules for source code, the coverage report from Jsfuzz is empty and not used +(and coverage guided fuzzing is also not available). When you discover a bug by fuzzing please keep the crash file. If you do not plan to fix the bug, either follow the [security policy] or file a [bug report] @@ -353,18 +335,35 @@ compatibility tests on all applicable Node.js versions. In the project's continuous integration the compatibility tests are run for all supported Node.js versions as well. +Test files in the compatibility test folder should correspond to the exported +package modules. + +When writing compatibility, keep in mind that the goal is to detect unsupported +language features, regardless of functional correctness. As such, the primary +goal is code coverage, not assertions. + +In general compatibility tests do not need to be updated. Only when a problem +occurred in practice that was not caught by the existing suite is it necessary +to update the tests. Of course, any improvements to the suite are welcome at any +point in time. + #### Breakage Testing The breakage tests aim to ensure that the API of the library isn't broken from release to release. All breakage tests go into the `test/breakage/` folder. You can run the breakage test using the command `npm run test:breakage`. -Breakage test compare both the API itself as well as the behavior of every -function of the API. This is achieved by depending on the latest version of the -library and using it in a differential test for each function in the API. +Test files in the breakage test folder should correspond to the exported package +modules. + +When writing breakage tests focus on verifying that the API itself as well as +the generic behavior of the API is unchanged when compared to the last release. +This comparison is achieved by depending on the latest version of the library +and using it to write differential test against the development head. -Unless the API is extended or a breaking API change is necessary this suite does -not need to be updated. +Unless the API is extended or a breaking API change is introduced this suite +does not need to be updated. Of course, any improvements to the suite are +welcome at any point in time. ### Writing Tests @@ -379,17 +378,29 @@ An oracle test checks an implementation against a given input-output pair (the oracle). This kind of test is useful for testing standard use cases, edge cases, regressions, or otherwise interesting cases. -For example, the maximum of the numbers `3` and `14` is `14`. +An example of an oracle test is one that asserts that the maximum of the numbers +`3` and `14` is `14`. ##### Property Tests A property test checks whether a given property holds, typically for many -inputs. A property can be many things and some properties have specific names -(e.g. an "invariant" property). +random inputs. A property can be many things, some named examples are listed +below. This kind of test is useful for creating confidence that the code being +tested is working for any input. + +An example of a property test is one that asserts that addition is commutative, +regardless of the numbers being added. + +###### Differential Tests + +A differential test checks that two similar functionalities behave the same. A +common use case is testing a known good implementation against a second +implementation. -For example, escaping an argument should always return a string value. +For example, this is used to test that the CommonJS version of the library +behaves the same as the original ESModule version of the library. -##### Metamorphic Tests +###### Metamorphic Tests A metamorphic test checks that similar action have similar results, without the need to know what exactly the result is. This kind of test can be useful to @@ -399,15 +410,6 @@ complex input-output relation. For example, escaping `N` arguments should have the same outcome as escaping `N-1` arguments and separately escaping the `N`th argument. -##### Differential Tests - -A differential test checks that two similar functionalities behave the same. A -common use case is testing a known good implementation against a second -implementation. - -For example, this is used to test that the CommonJS version of the library -behaves the same as the original ESModule version of the library. - #### Test Strategies ##### Simple Tests @@ -466,6 +468,17 @@ module.exports = { }; ``` +### Mutation testing + +The effectiveness of some test suites is ensured through [mutation testing +(Wikipedia)] using the [Stryker Mutator] framework. Mutation testing will tell +you if there are behaviour changing modification that can be made to source code +without the tests catching the change. Such modifications are labeled as +_survived_. + +This project has a zero survived mutants policy in order to ensure consistent +quality of test suites over time. + --- ## Documentation @@ -621,7 +634,8 @@ const john = "John Doe"; [markdown]: https://en.wikipedia.org/wiki/Markdown [markdownlint]: https://github.com/DavidAnson/markdownlint [mit license]: https://opensource.org/license/mit/ -[mutation testing]: https://en.wikipedia.org/wiki/Mutation_testing +[mutation testing]: #mutation-testing +[mutation testing (Wikipedia)]: https://en.wikipedia.org/wiki/Mutation_testing [node.js]: https://nodejs.org/en/ [npm]: https://www.npmjs.com/ [nve]: https://www.npmjs.com/package/nve @@ -631,5 +645,5 @@ const john = "John Doe"; [rollup.js]: https://rollupjs.org/guide/en/ [security policy]: ./SECURITY.md [shellcheck]: https://www.shellcheck.net/ -[stryker]: https://stryker-mutator.io/ +[stryker mutator]: https://stryker-mutator.io/ [typescript]: https://www.typescriptlang.org/ From 972f1463e1afcd7b3cf5efe1fa3ab16717886f8e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Oct 2023 08:09:06 +0200 Subject: [PATCH 2/3] Bump github/codeql-action from 2.22.2 to 2.22.4 (#1255) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2.22.2 to 2.22.4. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/d90b8d79de6dc1f58e83a1499aa58d6c93dc28de...49abf0ba24d0b7953cb586944e918a0b92074c80) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/checks.yml | 8 ++++---- .github/workflows/semgrep.yml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 6d298df5c..67ed0e518 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -76,12 +76,12 @@ jobs: - name: Checkout repository uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 - name: Initialize CodeQL - uses: github/codeql-action/init@d90b8d79de6dc1f58e83a1499aa58d6c93dc28de # v2.22.2 + uses: github/codeql-action/init@49abf0ba24d0b7953cb586944e918a0b92074c80 # v2.22.4 with: config-file: ./.github/codeql.yml languages: javascript - name: Perform CodeQL analysis - uses: github/codeql-action/analyze@d90b8d79de6dc1f58e83a1499aa58d6c93dc28de # v2.22.2 + uses: github/codeql-action/analyze@49abf0ba24d0b7953cb586944e918a0b92074c80 # v2.22.4 format: name: Formatting runs-on: ubuntu-22.04 @@ -214,7 +214,7 @@ jobs: with: args: . --sarif --output njsscan-results.sarif || true - name: Upload njsscan report to GitHub - uses: github/codeql-action/upload-sarif@d90b8d79de6dc1f58e83a1499aa58d6c93dc28de # v2.22.2 + uses: github/codeql-action/upload-sarif@49abf0ba24d0b7953cb586944e918a0b92074c80 # v2.22.4 if: ${{ failure() || success() }} with: sarif_file: njsscan-results.sarif @@ -613,7 +613,7 @@ jobs: scan-ref: . template: "@/contrib/sarif.tpl" - name: Upload Trivy report to GitHub - uses: github/codeql-action/upload-sarif@d90b8d79de6dc1f58e83a1499aa58d6c93dc28de # v2.22.2 + uses: github/codeql-action/upload-sarif@49abf0ba24d0b7953cb586944e918a0b92074c80 # v2.22.4 if: ${{ failure() || success() }} with: sarif_file: trivy-results.sarif diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index 2531bcab5..6414ca0c7 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -23,7 +23,7 @@ jobs: env: SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }} - name: Upload Semgrep report to GitHub - uses: github/codeql-action/upload-sarif@d90b8d79de6dc1f58e83a1499aa58d6c93dc28de # v2.22.2 + uses: github/codeql-action/upload-sarif@49abf0ba24d0b7953cb586944e918a0b92074c80 # v2.22.4 if: ${{ failure() || success() }} with: sarif_file: semgrep.sarif From 74f942859254ccc9c9eefba27b6d138f0df4ba8a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Oct 2023 10:46:48 +0200 Subject: [PATCH 3/3] Bump sinon from 16.1.0 to 17.0.0 (#1256) Bumps [sinon](https://github.com/sinonjs/sinon) from 16.1.0 to 17.0.0. - [Release notes](https://github.com/sinonjs/sinon/releases) - [Changelog](https://github.com/sinonjs/sinon/blob/main/docs/changelog.md) - [Commits](https://github.com/sinonjs/sinon/compare/v16.1.0...v17.0.0) --- updated-dependencies: - dependency-name: sinon dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 42 ++++++++++++++++++++++++++++++------------ package.json | 2 +- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 93ac09f26..e60721eed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40,7 +40,7 @@ "publint": "0.2.4", "rollup": "4.1.4", "shescape-previous": "npm:shescape@2.0.0", - "sinon": "16.1.0" + "sinon": "17.0.0" }, "engines": { "node": "^14.18.0 || ^16.13.0 || ^18 || ^19 || ^20" @@ -2179,9 +2179,9 @@ } }, "node_modules/@sinonjs/fake-timers": { - "version": "10.3.0", - "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-10.3.0.tgz", - "integrity": "sha512-V4BG07kuYSUkTCSBHG8G8TNhM+F19jXFWnQtzj+we8DrkpSBCee9Z3Ms8yiGer/dlmhe35/Xdgyo3/0rQKg7YA==", + "version": "11.2.2", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-11.2.2.tgz", + "integrity": "sha512-G2piCSxQ7oWOxwGSAyFHfPIsyeJGXYtc6mFbnFA+kRXkiEnTl8c/8jul2S329iFBnDI9HGoeWWAZvuvOkZccgw==", "dev": true, "dependencies": { "@sinonjs/commons": "^3.0.0" @@ -8682,9 +8682,9 @@ } }, "node_modules/nise": { - "version": "5.1.4", - "resolved": "https://registry.npmjs.org/nise/-/nise-5.1.4.tgz", - "integrity": "sha512-8+Ib8rRJ4L0o3kfmyVCL7gzrohyDe0cMFTBa2d364yIrEGMEoetznKJx899YxjybU6bL9SQkYPSBBs1gyYs8Xg==", + "version": "5.1.5", + "resolved": "https://registry.npmjs.org/nise/-/nise-5.1.5.tgz", + "integrity": "sha512-VJuPIfUFaXNRzETTQEEItTOP8Y171ijr+JLq42wHes3DiryR8vT+1TXQW/Rx8JNUhyYYWyIvjXTU6dOhJcs9Nw==", "dev": true, "dependencies": { "@sinonjs/commons": "^2.0.0", @@ -8703,6 +8703,24 @@ "type-detect": "4.0.8" } }, + "node_modules/nise/node_modules/@sinonjs/fake-timers": { + "version": "10.3.0", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-10.3.0.tgz", + "integrity": "sha512-V4BG07kuYSUkTCSBHG8G8TNhM+F19jXFWnQtzj+we8DrkpSBCee9Z3Ms8yiGer/dlmhe35/Xdgyo3/0rQKg7YA==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^3.0.0" + } + }, + "node_modules/nise/node_modules/@sinonjs/fake-timers/node_modules/@sinonjs/commons": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-3.0.0.tgz", + "integrity": "sha512-jXBtWAF4vmdNmZgD5FoKsVLv3rPgDnLgPbU84LIJ3otV44vJlDRokVng5v8NFJdCf/da9legHcKaRuZs4L7faA==", + "dev": true, + "dependencies": { + "type-detect": "4.0.8" + } + }, "node_modules/node-emoji": { "version": "1.11.0", "resolved": "https://registry.npmjs.org/node-emoji/-/node-emoji-1.11.0.tgz", @@ -11854,16 +11872,16 @@ } }, "node_modules/sinon": { - "version": "16.1.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-16.1.0.tgz", - "integrity": "sha512-ZSgzF0vwmoa8pq0GEynqfdnpEDyP1PkYmEChnkjW0Vyh8IDlyFEJ+fkMhCP0il6d5cJjPl2PUsnUSAuP5sttOQ==", + "version": "17.0.0", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-17.0.0.tgz", + "integrity": "sha512-p4lJiYKBoOEVUxxVIC9H1MM2znG1/c8gud++I2BauJA5hsz7hHsst35eurNWXTusBsIq66FzOQbZ/uMdpvbPIQ==", "dev": true, "dependencies": { "@sinonjs/commons": "^3.0.0", - "@sinonjs/fake-timers": "^10.3.0", + "@sinonjs/fake-timers": "^11.2.2", "@sinonjs/samsam": "^8.0.0", "diff": "^5.1.0", - "nise": "^5.1.4", + "nise": "^5.1.5", "supports-color": "^7.2.0" }, "funding": { diff --git a/package.json b/package.json index bb6f7c8a3..0bc7e1789 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "publint": "0.2.4", "rollup": "4.1.4", "shescape-previous": "npm:shescape@2.0.0", - "sinon": "16.1.0" + "sinon": "17.0.0" }, "scripts": { "prefuzz": "npm run transpile",