Expanded test coverage for pnpm lockfile processing #2004
Conversation
|
| @@ -23,3 +23,4 @@ node_modules/ | |||
| npm/private/test/node_modules/ | |||
| npm/private/test/npm_package/node_modules/ | |||
| npm/private/test/npm_package_publish/node_modules | |||
| .git | |||
There was a problem hiding this comment.
I don't think I've ever seen this in any bazel repo, do you have any more info on this?
There was a problem hiding this comment.
Bazel does not ignore .git by default, meaning commands like bazel build //... also search in .git for BUILD files.
This is bad for performance (especially in a large repository with a long history).
Additionally while rare, a .git directory could have a file Bazel will attempt to process. For example a branch called BUILD would create a file .git/refs/remotes/origin/BUILD containing the hash of the commit it current points to. Utter nonsense to Bazel that will fail the build/query/etc.
| } | ||
| } | ||
| """) | ||
| parsed_json = pnpm.parse_pnpm_lock_json(json.encode({ |
There was a problem hiding this comment.
Why switch to objects instead of JSON strings? When I'm writing these tests I always generate a pnpm-lock and then paste it into here, converting to json sounds like extra work when writing tests?
There was a problem hiding this comment.
Happy to switch this back.
I've forgotten the exact reason I did this, my guess is the syntax highlighting just made it easier to scan when comparing inputs to outputs. Could also just be habit, in a few internal Rust projects we use a json! macro so the data is expressed in the "host" language, meaning it'll be processed by existing tooling (e.g. formatters).
I do remember being annoyed that repr has no way of returning indented output.
There was a problem hiding this comment.
I'd prefer just switching it back. If you really want to debate it I'd rather debate it in a different PR so we can merge the rest of your changes sooner rather then later.
Split from #1924
This PR primarily serves to add test cases to cover the malformed package label generation that #1924 aims to fix, such that the changes in that other PR are more obvious.
Additionally;
.gitto.bazelignore(editors see.gitwhen inspecting test logs otherwise)file->findinnpm/private/npm_translate_lock_generate.bzl_new_package_infoinnpm/private/pnpm.bzl_convert_pnpm_v6_v9_version_peer_depinnpm/private/pnpm.bzljson.encodeinnpm/private/test/parse_pnpm_lock_tests.bzl//npm/private/test:test_parse_pnpm_lockChanges are visible to end-users: yes(-ish)
Test plan
NA, PR is adding new tests with little/no behaviour changes.