Node functional test improvements #9437
Conversation
fviernau
force-pushed
the
node-fun-test-improvements-iv
branch
from
6d9bb42 to
6b812b2
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
fviernau
force-pushed
the
node-fun-test-improvements-iv
branch
from
6b812b2 to
9c12e33
Compare
| @@ -1,6 +1,6 @@ | |||
| --- | |||
| project: | |||
| id: "NPM::<REPLACE_PROJECT_NAME>:1.0.0" | |||
| id: "NPM::npm-package-lock:1.0.0" | |||
There was a problem hiding this comment.
not from the filename of the definition file.
True, but what's a bit unclear from just looking at this diff is that the callers actually do set REPLACE_PROJECT_NAME to something based on the definitionFile's name.
So maybe better say:
Callers set
REPLACE_PROJECT_NAMEto something based on thedefinitionFile's name, which is wrong here as thenameof the identifier in the result comes from the string value of thenameproperty within thepackage.json.
| "resolve package-lock dependencies correctly" { | ||
| val definitionFile = getAssetFile("projects/synthetic/npm/package-lock/package.json") | ||
| val expectedResultFile = getAssetFile("projects/synthetic/npm/package-lock-expected-output.yml") | ||
| "resolve dependencies for a project with lockfile correctly" { |
There was a problem hiding this comment.
Nit: Maybe the commit message scope should be "npm" instead of "node" as a whole?
There was a problem hiding this comment.
This is actually true for other commits in this PR as well. Now that all Node PMs are separate, it could be beneficial to specify more narrow scopes named after the concrete PM.
| "cheerio": "1.0.0-rc.1", | ||
| "long": "^3.2.0", |
There was a problem hiding this comment.
Please explain in the commit message why these are reordered, and to which order. I guess to have alphabetical ordering, but why is that important here? Just for readability / consistency, or something else?
Maybe instead of "re-order" simply say "sort" to make things clearer?
| @@ -2,6 +2,8 @@ | |||
| project: | |||
| id: "PNPM::pnpm-package-with-lockfile:1.0.0" | |||
| definition_file_path: "<REPLACE_DEFINITION_FILE_PATH>" | |||
| authors: | |||
There was a problem hiding this comment.
Commit message typo in "comparision".
Callers set `REPLACE_PROJECT_NAME` to something based on the `definitionFile`'s name, which is wrong here as the name of the identifier in the result comes from the string value of the name property within the package.json. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Make the name consistent with similar projects for the other three node package managers. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Use alphabetical order for readability. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Align the dependencies of the `Yarn` and `pnpm` test projects with the
analog `Npm` one, using the respective `add` CLI command. This makes the
test results easier to compare.
Note: Doing this for `Yarn2` is not yet possible, because of
#9436.
Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Align the metadata with the one from `Npm` for a better comparison. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau
force-pushed
the
node-fun-test-improvements-iv
branch
from
9c12e33 to
205bcf5
Compare
Align the
project-with-lockfiletests, and add support for parsing the authors in Yarn2 projects.Part of #9261.