test: improve error message for policy failures - #35633
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for this! I'm sure we will both be happy to have these errors reported like this in CI rather than having to log into a failing CI host and poke around the file system.
Sorry, something went wrong.
|
Oh, sorry, but I forgot that this will also need an entry in test/common/README.md. It would go in the "Common module API" section. The entries are in alphabetical order. Maybe something like this? ### `requireNoPackageJSONAbove()`
Throws an `AssertionError` if a `package.json` file is in any ancestor
directory. Such files may interfere with proper test functionality. |
Sorry, something went wrong.
Trott
left a comment
There was a problem hiding this comment.
The changes to lib/internal/url.js look unrelated and accidental?
Sorry, something went wrong.
Co-authored-by: Rich Trott <rtrott@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
Since they were pretty clearly unrelated and accidental, I rebased and added a fixup commit to remove them. |
Sorry, something went wrong.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #35633 +/- ##
=======================================
Coverage 96.40% 96.40%
=======================================
Files 220 220
Lines 73681 73681
=======================================
+ Hits 71031 71034 +3
+ Misses 2650 2647 -3
Continue to review full report at Codecov.
|
Sorry, something went wrong.
Sorry, something went wrong.
Policy tests can fail if a `package.json` exists in any of the parent directories above the test. The existing checks are done for the ancestors of the test directory but some tests execute from the tmpdir. PR-URL: #38285 Refs: #38088 Refs: #35600 Refs: #35633 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Policy tests can fail if a `package.json` exists in any of the parent directories above the test. The existing checks are done for the ancestors of the test directory but some tests execute from the tmpdir. PR-URL: #38285 Refs: #38088 Refs: #35600 Refs: #35633 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Policy tests can fail if a `package.json` exists in any of the parent directories above the test. The existing checks are done for the ancestors of the test directory but some tests execute from the tmpdir. PR-URL: #38285 Refs: #38088 Refs: #35600 Refs: #35633 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Policy tests can fail if a `package.json` exists in any of the parent directories above the test. The existing checks are done for the ancestors of the test directory but some tests execute from the tmpdir. PR-URL: #38285 Refs: #38088 Refs: #35600 Refs: #35633 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Policy tests can fail if a `package.json` exists in any of the parent directories above the test. The existing checks are done for the ancestors of the test directory but some tests execute from the tmpdir. PR-URL: #38285 Refs: #38088 Refs: #35600 Refs: #35633 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
fixes: #35600
this doesn't attempt to fix this outside of running node's test suite since we cannot know when a package.json is expected or not except through the policy itself. in theory any test relying on "type" should also use this helper... but that is basically everything so it is fine to omit I think.