Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(npm): don't break on npm workspace dir with a period #30483

Merged
merged 4 commits into from
Jul 30, 2024

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Jul 30, 2024

When updating from the root folder, lockFileDir is '.'. The previous
path normalization would break an npm workspaceDir that happened to have
a period in it (e.g. "packages/foo.bar"), resulting in the
npm install ... --workspace=... command breaking with:

npm error No workspaces found:\nnpm error   --workspace=packages/foobar

Fixes: #30466

When updating from the root folder, `lockFileDir` is '.'. The previous
path normalization would break an npm workspaceDir that happened to have
a period in it (e.g. "packages/foo.bar"), resulting in the
`npm install ... --workspace=...` command breaking with:

    npm error No workspaces found:\nnpm error   --workspace=packages/foobar

Fixes: renovatebot#30466
@CLAassistant
Copy link

CLAassistant commented Jul 30, 2024

CLA assistant check
All committers have signed the CLA.

@trentm
Copy link
Contributor Author

trentm commented Jul 30, 2024

Here is example test output when running the added test without the fix to prove that the test is testing what it intends:

% pnpm jest --collectCoverage=false lib/modules/manager/npm/post-update

> [email protected] jest /Users/trentm/tm/renovate2
> GIT_ALLOW_PROTOCOL=file LOG_LEVEL=fatal node --experimental-vm-modules node_modules/jest/bin/jest.js --logHeapUsage "--collectCoverage=false" "lib/modules/manager/npm/post-update"

Host stats:
    Cpus:      11
    Memory:    36.00 GB
    HeapLimit: 4.05 GB
 PASS  lib/modules/manager/npm/post-update/rules.spec.ts (52 MB heap size)
 PASS  lib/modules/manager/npm/post-update/yarn.spec.ts (116 MB heap size)
 PASS  lib/modules/manager/npm/post-update/node-version.spec.ts (159 MB heap size)
 PASS  lib/modules/manager/npm/post-update/pnpm.spec.ts (166 MB heap size)
 FAIL  lib/modules/manager/npm/post-update/npm.spec.ts (172 MB heap size)
  ● modules/manager/npm/post-update/npm › installs workspace only packages separately › workspace in root folder

    expect(received).toMatchObject(expected)

    - Expected  - 1
    + Received  + 1

    @@ -4,11 +4,11 @@
        },
        Object {
          "cmd": "npm install --package-lock-only --no-audit --ignore-scripts --workspace=web/b [email protected] [email protected]",
        },
        Object {
    -     "cmd": "npm install --package-lock-only --no-audit --ignore-scripts --workspace=docs/dir.has.period [email protected]",
    +     "cmd": "npm install --package-lock-only --no-audit --ignore-scripts --workspace=docs/dirhas.period [email protected]",
        },
        Object {
          "cmd": "npm install --package-lock-only --no-audit --ignore-scripts [email protected] [email protected]",
        },
      ]

      592 |       expect(fs.readLocalFile).toHaveBeenCalledTimes(3);
      593 |       expect(res.error).toBeFalse();
    > 594 |       expect(execSnapshots).toMatchObject([
          |                             ^
      595 |         {
      596 |           cmd: 'npm install --package-lock-only --no-audit --ignore-scripts --workspace=docs/a [email protected] [email protected]',
      597 |         },

      at Object.<anonymous> (lib/modules/manager/npm/post-update/npm.spec.ts:594:29)

 PASS  lib/modules/manager/npm/post-update/index.spec.ts (354 MB heap size)

Test Suites: 1 failed, 5 passed, 6 total
Tests:       1 failed, 129 passed, 130 total
Snapshots:   36 passed, 36 total
Time:        3.879 s, estimated 4 s
Ran all test suites matching /lib\/modules\/manager\/npm\/post-update/i.
 ELIFECYCLE  Command failed with exit code 1.

rarkins
rarkins previously approved these changes Jul 30, 2024
@rarkins rarkins enabled auto-merge July 30, 2024 18:21
@rarkins rarkins added this pull request to the merge queue Jul 30, 2024
Merged via the queue into renovatebot:main with commit 5bdaf47 Jul 30, 2024
38 checks passed
@trentm trentm deleted the tm-fix-npm-workspace-dir-with-period branch July 30, 2024 18:31
@trentm
Copy link
Contributor Author

trentm commented Jul 30, 2024

Thanks!

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 38.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants