* test: replace manual process.env mutation with vi.stubEnv
Enable Vitest's `unstubEnvs` so env stubs are reverted automatically
before every test, and convert the manual `process.env.X = ...` /
`delete process.env.X` pairs in specs to `vi.stubEnv()`.
This removes the hand-written `afterEach` cleanup blocks and the
`const OLD_ENV = process.env` / `process.env = { ...OLD_ENV }`
save-restore dance, which silently leaked state between tests when a
test set a variable without a matching delete.
Left as-is (incompatible with `unstubEnvs`, which unstubs before each
test, not after):
- module top-level env writes (cocoapods, gomod, post-update, pip-compile
and bundler artifacts specs)
- `beforeAll` env writes -- the two in `util/http` moved to `beforeEach`
* test: stop asserting on ambient environment variables
Three specs asserted on values that the surrounding environment can
override, so they only passed on a machine that happened not to set them:
- `util/exec/env`: `getChildProcessEnv()` forwards every entry of
`basicEnvVars`, but the spec only seeded a subset. Any other forwarded
variable that was set ambiently (e.g. a lowercase `http_proxy`) leaked
into the `toEqual` assertion. Clear all of them first.
- `util/git/index`: `process.env.GIT_SSH_COMMAND` deliberately overrides
the default that the two env-inheritance tests assert on. Clear it.
- `logger/pretty-stdout`: the colorized level strings are built at import
time, so the `beforeEach` setting `FORCE_COLOR` could never affect them
-- it was dead code, and the test silently assumed a terminal without
colour support. Derive the expectation the same way the module does.
* test(datasource/nuget): use a throwaway cache dir for nupkg downloads
The `determine source URL from nupkg` tests do not mock the filesystem,
so they really wrote downloaded packages into a fixed `/tmp/cache` path.
That fails wherever the system tmpdir is not writable, and two checkouts
running the suite at once would share the same directory.
Use `tmp.dir({ unsafeCleanup: true })` and clean it up afterwards, which
is the pattern the other specs that touch disk already use.
* feat(lint): add renovate/prefer-stub-env rule
Enforces in specs what the migration to `vi.stubEnv` established:
- assigning to or deleting `process.env.X` inside `beforeEach`, `afterEach`,
`it` or `test` is reported, with a fixer that rewrites it to `vi.stubEnv()`
- module scope, `describe` bodies and `beforeAll` / `afterAll` are left alone:
`unstubEnvs` reverts stubs *before* each test, so a stub created there is
gone by the time the first test runs, and those must keep assigning directly
- replacing the `process.env` object is reported everywhere and never fixed.
`vi.stubEnv()` captures the original object when the worker starts, so after
a reassignment its deletes silently target the old object
The rule found 28 further violations in specs the earlier sweep missed, since
that one was anchored on `delete process.env`. Twelve were auto-fixed; the
rest replaced `process.env` wholesale and are now `clearEnv()` from test/util.
`util/exec/index.spec.ts` opts out with a comment: it asserts on the whole
child env, and `vi.stubEnv()` cannot express that because it maps `PROD`,
`DEV` and `SSR` onto '' instead of deleting them.
* refactor(oxlint): enable func-style
Enable the built-in func-style rule as ["warn", "declaration"] so that
named functions are written as function declarations instead of arrow or
function expressions assigned to variables, matching the convention in
docs/development/best-practices.md (Functions section). Inline arrow
callbacks are unaffected; prefer-arrow-callback continues to cover the
callback direction.
Severity is "warn" instead of "error" as a fallback: a repo-wide run
found 196 pre-existing violations (70 in spec files, 126 in production
code), exceeding the 80-violation fix threshold for this change, so no
violations were fixed here. Follow-up work can convert the offenders
(largest clusters: lib/modules/versioning/ruby, lib/util/cache/package,
lib/modules/versioning/maven, lib/modules/versioning/gradle) and then
raise the rule to "error". Note oxlint 1.72 also supports an
allowTypeAnnotation option if typed consts should remain arrows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci(linter): enforce func-style at error
* refactor(func-style): convert const-arrow functions to declarations in with-cache spec and ruby versioning
* refactor(func-style): convert const-arrow functions to declarations in maven/gradle modules
* refactor(func-style): convert const-arrow functions to declarations in remaining versioning modules
* refactor(func-style): convert const-arrow functions to declarations in platform modules
Discriminated-union closures (ensureCommentRemoval byTopic/byContent
helpers in github, gitlab, bitbucket, bitbucket-server) needed the
narrowed field passed explicitly as a parameter, since function
declarations are hoisted and lose the outer if-narrowing that inline
arrow-function closures previously relied on.
* refactor(func-style): convert const-arrow functions to declarations across util/manager/datasource modules
One case (logger/renovate-logger.ts logOnceFn) is kept as an arrow
function with a documented oxlint-disable: it is stored on
this.logger.once[level] and relies on lexically-bound this, which a
hoisted function declaration would not preserve.
* refactor(func-style): convert remaining const-arrow test helpers to declarations
get-updated.spec.ts: hoisted expectPipCompilePackageAndLockFile to
module scope (it only closes over the module-level pipCompile mock)
and added a documented oxlint-disable for vitest/expect-expect on its
caller test, since oxlint's expect-expect check does not trace
assertions through an externally-defined helper function.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* refactor(lint): extract custom oxlint rules into separate files
Split `tools/lint/rules.js` into one file per rule under
`tools/lint/rules/`. Also simplify `test-root-describe` with
early-exit guard clauses.
* feat(lint): add `enforce-ts-extension` oxlint rule
Add a custom JS-based oxlint rule that enforces `.ts` extensions on
local imports. The rule catches two problems:
- `.js` extension used instead of `.ts` (auto-fixable)
- missing file extension entirely (report only)
Checked locations:
- static import/export declarations
- dynamic `import()` expressions
- vitest module APIs: `vi.mock`, `vi.doMock`, `vi.unmock`,
`vi.doUnmock`, `vi.importActual`, `vi.importMock`
Both relative paths (`./`, `../`) and path aliases (`~test/`)
are treated as local. Package imports are ignored.
Also fixes 26 pre-existing violations across 18 spec files.
* Update tools/lint/rules/enforce-ts-extension.js
Co-authored-by: Anne Stellingwerf <1637358+astellingwerf@users.noreply.github.com>
---------
Co-authored-by: Anne Stellingwerf <1637358+astellingwerf@users.noreply.github.com>
As part of future changes, we need to be able to specify on a
per-command level whether a `shell` should be used.
Before we do this, we should make it possible to provide the `shell`
specifier.
As part of future changes which will fix issues with Composer + Yarn 1
introduced by f430552de4, we need to find
a way to ignore command failure, on a per-command basis.
To do this, we can provide a refactor to introduce a new type,
`CommandWithOptions` which has the ability to specify whether to
`ignoreFailure`.
To make this less of a refactor than would otherwise require, we can
pass around a `(string | CommandWithOptions)[]`
(or a `string[]` or a `CommandWithOptions[]`) to be used, and use
helpers `asRawCommand` and `asRawCommands` to extract the underlying
command where necessary.
In the case a command's failure is being ignored, return the error code
in the `ExecResult`.
We also need to make sure that when executing in Docker, we still
provide the `|| true`s, otherwise we'll start to see commands failing
unexpectedly.
As part of future-proofing the `CommandWithOptions`, we can make sure
that it uses `string[]`s for the command it's representing, to avoid
re-parsing it.
As part of a future change, we will optionally be passing around
command arguments.
To make this a simpler change, we can prefactor this to pass around the
(empty) args array first.
* fix(util/exec): migrate to `execa` for command execution
As part of a future change, we need to make sure that we have
cross-platform support for specific command executions.
By using `execa`'s functionality on top of `node:child_process`, we can
provide this functionality, as well as retain a consistent API with
internal users.
We need to make a couple of small tweaks to setting the `RawExecOptions`
as we're now bound by `readonly` properties.
As the `stdio` property's type is not exported, we can replace this with
an explicit setting of `stdin`, `stdout` and `stderr`.
This requires a number of test changes to make sure that the `stdin`,
`stdout` and `stderr` are set.
This doesn't migrate any calls under `tools`, as they can remain using
`node:child_process`.
* chore(util/exec): ignore floating promises `.on` calls
Since moving to use `execa`, we now have "floating promises" for the
`.on` method calls.
As we aren't handling them here, we can now mark them as `void` (as an
alternative to ignoring the ESLint rule).
This has been deprecated since 2022-07, and is unused as we only ever
use UTF-8.
In `tools/utils/exec.ts` we make sure to specify it explicitly, so we
don't get a Buffer returned.
As part of ongoing work to improve OpenTelemetry instrumentation of
Renovate in #38609, we can introduce an instrumented call for each
external command execution.
This makes sure we cover the key functions used for command
execution - `rawExec` - across the codebase, which is used
under-the-hood by `exec`.
Because commands could include sensitive arguments (such as repo or
global secrets) we need to make sure we sanitize the span name.
Note that we don't add instrumentation to `tools/utils/exec.ts` as it's
only used for i.e. building the docs site, and isn't used as part of the
core Renovate CLI.
* refactor(util): use spawn instead of exec
- Use child_process.spawn instead of child_process.exec
* refactor(util): use spawn instead of exec
- Use child_process.spawn instead of child_process.exec
* refactor(util): use spawn instead of exec
- Use child_process.spawn instead of child_process.exec
* refactor(util): use spawn instead of exec
- Use child_process.spawn instead of child_process.exec
* refactor(util): use spawn instead of exec
- init spawn-util
* refactor(util): use spawn instead of exec
- spawn-util
* refactor(util): use spawn instead of exec
- init index-spawn.spec.ts
* refactor(util): use spawn instead of exec
- fixed various tests
* refactor(util): use spawn instead of exec
- fix all artifacts.spec.ts
* refactor(util): use spawn instead of exec
- fix all artifacts.spec.ts
* refactor(util): use spawn instead of exec
- fix npm post update imports
* refactor(util): use spawn instead of exec
- revert renaming to minimize PR diff
* refactor(util): use spawn instead of exec
- revert renaming to minimize PR diff
* refactor(util): use spawn instead of exec
- revert renaming to minimize PR diff
* refactor(util): use spawn instead of exec
- revert renaming to minimize PR diff
* refactor(util): use spawn instead of exec
- revert renaming to minimize PR diff
- destroy stdio when terminating child process
* refactor(util): use spawn instead of exec
- delete and revert dev related changes
* refactor(util): use spawn instead of exec
- fix support for windows
* refactor(util): use spawn instead of exec
- handle SIGSTOP and such
- add test coverage
* refactor(util): use spawn instead of exec
- now converts to strings when resolving/rejecting
* refactor(util): use spawn instead of exec
- logs improvements
- force shell (exec like)
- fix tests
* refactor(util): use spawn instead of exec
- strongly type listeners
* refactor(util): use spawn instead of exec
- create helper mock for spawn
* refactor(util): use spawn instead of exec
- cr changes
* Update lib/util/exec/common.ts
Co-authored-by: Sergei Zharinov <zharinov@users.noreply.github.com>
* refactor(util): use spawn instead of exec
- documentation
* refactor(util): use spawn instead of exec
- revert unnecessary formatting
* refactor(util): use spawn instead of exec
* refactor(util): use spawn instead of exec
- added ExecError class
* refactor(util): use spawn instead of exec
- exec-error.ts restructure
* refactor(util): use spawn instead of exec
* Apply suggestions from code review
Co-authored-by: Sergei Zharinov <zharinov@users.noreply.github.com>
* refactor(util): use spawn instead of exec
* refactor(util): use spawn instead of exec
* refactor(util): use spawn instead of exec
- deprecated RawExecOptions.encoding property
* refactor(util): use spawn instead of exec
* refactor(util): use spawn instead of exec
* refactor(util): use spawn instead of exec
Co-authored-by: Sergei Zharinov <zharinov@users.noreply.github.com>
Michael Kriese