Skip to content

Runs more tests #5875

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.

Sign up for GitHub

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

Merged
arcanis merged 3 commits into from
Oct 26, 2023
Merged

Runs more tests #5875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
651ee2d
Runs more tests
arcanis Oct 26, 2023
09f43a8
Fixes tests
arcanis Oct 26, 2023
723dc33
Adds tests suitable for Node 21
arcanis Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/workflows/integration-workflow.yml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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]);
Copy link
Member

@merceyz merceyz Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think adding this here is necessary when it's only used in tests.

Copy link
Member Author

@arcanis arcanis Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still prefer it to duplicating the code, and nodeUtils is as good a place as any. I'd prefer to avoid adding test-specific "general utils", as they are harder to discover.


const openUrlBinary = new Map([
[`darwin`, `open`],
[`linux`, `xdg-open`],
Expand Down
36 changes: 31 additions & 5 deletions packages/yarnpkg-fslib/sources/NodeFS.ts
View file
Open in desktop
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