Skip to content

Commit

Permalink
Runs more tests (#5875)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**

We don't run the unit tests on Node 20/21 on other OSes than Linux, so
we didn't notice that one of them was
[failing](https://github.com/nodejs/citgm/actions/runs/6652341543/job/18076121495#step:5:146).

**How did you fix it?**

This diff increases the coverage so we test both the oldest and newest
supported releases across all platforms. It also adds Node.js 21, to
track regressions before they land into an even release.

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
  • Loading branch information
arcanis authored Oct 26, 2023
1 parent ddf08d6 commit 36b3c2d
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 10 deletions.
13 changes: 11 additions & 2 deletions .github/workflows/integration-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,11 @@ jobs:
fail-fast: false
matrix:
# We run the ubuntu tests on multiple Node versions with 2 shards since they're the fastest.
node: [18, 19, 20]
node: [18, 19, 20, 21]
platform: [[ubuntu, 20.04]]
shard: ['1/2', '2/2']
# We run the rest of the tests on the minimum Node version we support with 3 shards.
include:
# We run the rest of the tests on the minimum Node version we support with 3 shards.
# Windows tests
- {node: 18, platform: [windows, latest], shard: 1/3}
- {node: 18, platform: [windows, latest], shard: 2/3}
Expand All @@ -214,6 +214,15 @@ jobs:
- {node: 18, platform: [macos, latest], shard: 1/3}
- {node: 18, platform: [macos, latest], shard: 2/3}
- {node: 18, platform: [macos, latest], shard: 3/3}
# We also run them on the maximum Node version we support, to catch potential regressions in Node.js.
# Windows tests
- {node: 21, platform: [windows, latest], shard: 1/3}
- {node: 21, platform: [windows, latest], shard: 2/3}
- {node: 21, platform: [windows, latest], shard: 3/3}
# macOS tests
- {node: 21, platform: [macos, latest], shard: 1/3}
- {node: 21, platform: [macos, latest], shard: 2/3}
- {node: 21, platform: [macos, latest], shard: 3/3}

name: '${{matrix.platform[0]}}-latest w/ Node.js ${{matrix.node}}.x (${{matrix.shard}})'
runs-on: ${{matrix.platform[0]}}-${{matrix.platform[1]}}
Expand Down
39 changes: 39 additions & 0 deletions .yarn/versions/9d9fcf70.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/core": patch
"@yarnpkg/fslib": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-exec"
- "@yarnpkg/plugin-file"
- "@yarnpkg/plugin-git"
- "@yarnpkg/plugin-github"
- "@yarnpkg/plugin-http"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-link"
- "@yarnpkg/plugin-nm"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- vscode-zipfs
- "@yarnpkg/builder"
- "@yarnpkg/doctor"
- "@yarnpkg/extensions"
- "@yarnpkg/libzip"
- "@yarnpkg/nm"
- "@yarnpkg/pnp"
- "@yarnpkg/pnpify"
- "@yarnpkg/sdks"
- "@yarnpkg/shell"
51 changes: 48 additions & 3 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import {nodeUtils} from '@yarnpkg/core';
import {Filename, npath, ppath, xfs} from '@yarnpkg/fslib';
import {pathToFileURL} from 'url';

const ifAtLeastNode21It = nodeUtils.major >= 21 ? it : it.skip;
const ifAtMostNode20It = nodeUtils.major <= 20 ? it : it.skip;

describe(`Plug'n'Play - ESM`, () => {
test(
`it should be able to import a node builtin`,
Expand Down Expand Up @@ -399,7 +403,7 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

test(
ifAtMostNode20It(
`it should not allow extensionless commonjs imports`,
makeTemporaryEnv(
{ },
Expand All @@ -420,7 +424,27 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

test(
ifAtLeastNode21It(
`it should allow extensionless commonjs imports`,
makeTemporaryEnv(
{ },
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index.mjs`), `import bin from './cjs-bin';\nconsole.log(bin)`);
await xfs.writeFilePromise(ppath.join(path, `cjs-bin`), `module.exports = 42`);

await expect(run(`install`)).resolves.toMatchObject({code: 0});

await expect(run(`node`, `./index.mjs`)).resolves.toMatchObject({
stdout: `42\n`,
});
},
),
);

ifAtMostNode20It(
`it should not allow extensionless files with {"type": "module"}`,
makeTemporaryEnv(
{
Expand All @@ -430,7 +454,7 @@ describe(`Plug'n'Play - ESM`, () => {
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index`), ``);
await xfs.writeFilePromise(ppath.join(path, `index`), `console.log(42)`);

await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand All @@ -442,6 +466,27 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

ifAtLeastNode21It(
`it should allow extensionless files with {"type": "module"}`,
makeTemporaryEnv(
{
type: `module`,
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index`), `console.log(42)`);

await expect(run(`install`)).resolves.toMatchObject({code: 0});

await expect(run(`node`, `./index`)).resolves.toMatchObject({
stdout: `42\n`,
});
},
),
);

test(
`it should support ESM binaries`,
makeTemporaryEnv(
Expand Down
2 changes: 2 additions & 0 deletions packages/yarnpkg-core/sources/nodeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import os from 'os';
import * as execUtils from './execUtils';
import * as miscUtils from './miscUtils';

export const major = Number(process.versions.node.split(`.`)[0]);

const openUrlBinary = new Map([
[`darwin`, `open`],
[`linux`, `xdg-open`],
Expand Down
36 changes: 31 additions & 5 deletions packages/yarnpkg-fslib/sources/NodeFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,17 @@ import {CreateReadStreamOptions, CreateWriteStreamOptions, Dir, StatWatcher, Wat
import {Dirent, SymlinkType, StatSyncOptions, StatOptions} from './FakeFS';
import {BasePortableFakeFS, WriteFileOptions} from './FakeFS';
import {MkdirOptions, RmdirOptions, WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {FSPath, PortablePath, Filename, ppath, npath} from './path';
import {FSPath, PortablePath, Filename, ppath, npath, NativePath} from './path';

function direntToPortable(dirent: Dirent<NativePath>): Dirent<PortablePath> {
// We don't need to return a copy, we can just reuse the object the real fs returned
const portableDirent = dirent as Dirent<PortablePath>;

if (typeof dirent.path === `string`)
portableDirent.path = npath.toPortablePath(dirent.path);

return portableDirent;
}

export class NodeFS extends BasePortableFakeFS {
private readonly realFs: typeof fs;
Expand Down Expand Up @@ -469,9 +479,17 @@ export class NodeFS extends BasePortableFakeFS {
async readdirPromise(p: PortablePath, opts?: ReaddirOptions | null): Promise<Array<Dirent<PortablePath> | DirentNoPath | PortablePath>> {
return await new Promise<any>((resolve, reject) => {
if (opts) {
this.realFs.readdir(npath.fromPortablePath(p), opts as any, this.makeCallback(resolve, reject) as any);
if (opts.recursive && process.platform === `win32`) {
if (opts.withFileTypes) {
this.realFs.readdir(npath.fromPortablePath(p), opts as any, this.makeCallback<Array<Dirent<NativePath>>>(results => resolve(results.map(direntToPortable)), reject) as any);
} else {
this.realFs.readdir(npath.fromPortablePath(p), opts as any, this.makeCallback<Array<NativePath>>(results => resolve(results.map(npath.toPortablePath)), reject) as any);
}
} else {
this.realFs.readdir(npath.fromPortablePath(p), opts as any, this.makeCallback(resolve, reject) as any);
}
} else {
this.realFs.readdir(npath.fromPortablePath(p), this.makeCallback(value => resolve(value as Array<Filename>), reject));
this.realFs.readdir(npath.fromPortablePath(p), this.makeCallback(resolve, reject));
}
});
}
Expand All @@ -488,9 +506,17 @@ export class NodeFS extends BasePortableFakeFS {
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes: boolean}): Array<Dirent<PortablePath> | DirentNoPath | PortablePath>;
readdirSync(p: PortablePath, opts?: ReaddirOptions | null): Array<Dirent<PortablePath> | DirentNoPath | PortablePath> {
if (opts) {
return this.realFs.readdirSync(npath.fromPortablePath(p), opts as any) as Array<any>;
if (opts.recursive && process.platform === `win32`) {
if (opts.withFileTypes) {
return (this.realFs.readdirSync(npath.fromPortablePath(p), opts as any) as any as Array<Dirent<NativePath>>).map(direntToPortable);
} else {
return (this.realFs.readdirSync(npath.fromPortablePath(p), opts as any) as any as Array<NativePath>).map(npath.toPortablePath);
}
} else {
return this.realFs.readdirSync(npath.fromPortablePath(p), opts as any) as Array<PortablePath>;
}
} else {
return this.realFs.readdirSync(npath.fromPortablePath(p)) as Array<any>;
return this.realFs.readdirSync(npath.fromPortablePath(p)) as Array<PortablePath>;
}
}

Expand Down

0 comments on commit 36b3c2d

Please sign in to comment.