-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
chore: add initial ecosystem plugin tests workflow #19643
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
578c12d to
baa84df
Compare
| on: | ||
| push: | ||
| branches: | ||
| - "ecosystem/*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Feature] I hadn't thought of having a branch wildcard in the RFC. But I think it'd be nice if we want to have branches/PRs that explicitly test ecosystem logic. Especially for updating this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand the use case you're envisioning here. Can you explain more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that if someone wants to work on the ecosystem flow itself, it would be inconvenient to have to always manually dispatch the workflow. This branches setting means the flow will run on every push to a branch with a name matching ecosystem/*.
nzakas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting started on this. Left some comments throughout.
|
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
Co-authored-by: Nicholas C. Zakas <[email protected]>
|
Todo for self: noting https://github.com/eslint/eslint/pull/19643/files#r2068835929 at root level so I don't forget to file the issue. |
|
Just pushed a commit adding |
fasttime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. When I run the test-ecosystem tool with DEBUG=test:ecosystem, I can see that ni stops on the prompt "Choose the agent". I'm getting the same results on Windows and MacOS.
$ DEBUG=test:ecosystem npm run test:ecosystem
> [email protected] test:ecosystem
> node tools/test-ecosystem/index.mjs
Plugins to test: @eslint/css, @eslint/json, @eslint/markdown, @eslint-community/eslint-plugin-eslint-comments, @typescript-eslint/typescript-eslint, eslint-plugin-unicorn, eslint-plugin-vue
Clearing existing sandbox directory: .../eslint/ecosystem
Testing @eslint/css in .../eslint/ecosystem/eslint-css
[@eslint/css] git clone https://github.com/eslint/css .../eslint/ecosystem/eslint-css --depth 1
Cloning into '.../eslint/ecosystem/eslint-css'...
remote: Enumerating objects: 117, done.
remote: Counting objects: 100% (117/117), done.
remote: Compressing objects: 100% (109/109), done.
remote: Total 117 (delta 10), reused 50 (delta 2), pack-reused 0 (from 0)
Receiving objects: 100% (117/117), 117.95 KiB | 3.28 MiB/s, done.
Resolving deltas: 100% (10/10), done.
[@eslint/css] git fetch origin 26b902c4f42cccb33d7f8119a3376773e0ad91bd
remote: Enumerating objects: 1083, done.
remote: Counting objects: 100% (1083/1083), done.
remote: Compressing objects: 100% (340/340), done.
remote: Total 1007 (delta 689), reused 929 (delta 613), pack-reused 0 (from 0)
Receiving objects: 100% (1007/1007), 237.74 KiB | 5.17 MiB/s, done.
Resolving deltas: 100% (689/689), completed with 56 local objects.
From https://github.com/eslint/css
* branch 26b902c4f42cccb33d7f8119a3376773e0ad91bd -> FETCH_HEAD
[@eslint/css] git checkout 26b902c4f42cccb33d7f8119a3376773e0ad91bd
Note: switching to '26b902c4f42cccb33d7f8119a3376773e0ad91bd'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 26b902c feat: add @namespace validation to no-invalid-at-rule-placement rule (#183)
[@eslint/css] pwd
.../eslint/ecosystem/eslint-css
[@eslint/css] ni
? Choose the agent › - Use arrow-keys. Return to submit.
❯ npm
yarn
pnpm
bun
deno
Is anyone else getting the same results?
|
@fasttime I was not able to repro on windows+ ps. |
|
I'm using Windows 11, and when I run
|
|
Interesting. There is an ~undocumented I added it in 862dd99 just now. Can you try again? |
|
Better to upgrade |
Co-authored-by: 唯然 <[email protected]>
|
Oh @lumirlumir I don't think it existed in the version of ni we were using - can you try again? |
fasttime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ni recommends using a lockfile so it can automatically detect which package manager to use. From the documentation:
ni assumes that you work with lock-files (and you should).
Before
niruns the command, it detects youryarn.lock/pnpm-lock.yaml/package-lock.json/bun.lock/bun.lockb/deno.json/deno.jsoncto know the current package manager (orpackageManagerfield in your packages.json if specified) using the package-manager-detector package and then runs the corresponding package-manager-detector command.
It looks like the prompt asking you to choose a package manager only appears when ni cannot detect one automatically.
On my machine, when I set NI_DEFAULT_AGENT=npm, npm is used automatically for projects without a lockfile such as @eslint/css, so no prompt is shown and npm becomes the fallback:
Details
$ DEBUG=test:ecosystem NI_DEFAULT_AGENT=npm npm run test:ecosystem
> [email protected] test:ecosystem
> node tools/test-ecosystem/index.mjs
Plugins to test: @eslint/css, @eslint/json, @eslint/markdown, @eslint-community/eslint-plugin-eslint-comments, @typescript-eslint/typescript-eslint, eslint-plugin-unicorn, eslint-plugin-vue
Clearing existing sandbox directory: .../eslint/ecosystem
Testing @eslint/css in .../eslint/ecosystem/eslint-css
[@eslint/css] git clone https://github.com/eslint/css .../eslint/ecosystem/eslint-css --depth 1
Cloning into '.../eslint/ecosystem/eslint-css'...
[@eslint/css] git fetch origin 26b902c4f42cccb33d7f8119a3376773e0ad91bd
From https://github.com/eslint/css
* branch 26b902c4f42cccb33d7f8119a3376773e0ad91bd -> FETCH_HEAD
[@eslint/css] git checkout 26b902c4f42cccb33d7f8119a3376773e0ad91bd
Note: switching to '26b902c4f42cccb33d7f8119a3376773e0ad91bd'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 26b902c feat: add @namespace validation to no-invalid-at-rule-placement rule (#183)
[@eslint/css] pwd
.../eslint/ecosystem/eslint-css
[@eslint/css] ni
npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
> @eslint/[email protected] prepare
> npm run build
> @eslint/[email protected] build
> npm run build:rules && rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && npm run build:cts
> @eslint/[email protected] build:rules
> node tools/build-rules.js
Recommended rules generated successfully.
Rules import file generated successfully.
src/index.js → dist/cjs/index.cjs, dist/esm/index.js...
(!) Unresolved dependencies
https://rollupjs.org/troubleshooting/#warning-treating-module-as-external-dependency
@eslint/css-tree (imported by "src/languages/css-language.js")
@eslint/plugin-kit (imported by "src/languages/css-source-code.js")
(!) Mixing named and default exports
https://rollupjs.org/configuration-options/#output-exports
The following entry modules are using named and default exports together:
src/index.js
Consumers of your bundle will have to use chunk.default to access their default export, which may not be what you want. Use `output.exports: "named"` to disable this warning.
created dist/cjs/index.cjs, dist/esm/index.js in 64ms
> @eslint/[email protected] build:dedupe-types
> node tools/dedupe-types.js dist/cjs/index.cjs dist/esm/index.js
> @eslint/[email protected] build:cts
> node -e "fs.copyFileSync('dist/esm/index.d.ts', 'dist/cjs/index.d.cts')" && node tools/update-cts.js dist/cjs/types.cts dist/cjs/index.d.cts
added 436 packages, and audited 437 packages in 3s
150 packages are looking for funding
run `npm fund` for details
3 high severity vulnerabilities
Some issues need review, and may require choosing
a different dependency.
Run `npm audit` for details.
[@eslint/css] npm link eslint
removed 28 packages, changed 1 package, and audited 410 packages in 679ms
144 packages are looking for funding
run `npm fund` for details
3 high severity vulnerabilities
Some issues need review, and may require choosing
a different dependency.
Run `npm audit` for details.
[@eslint/css] nr build
> @eslint/[email protected] build
> npm run build:rules && rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && npm run build:cts
> @eslint/[email protected] build:rules
> node tools/build-rules.js
Recommended rules generated successfully.
Rules import file generated successfully.
src/index.js → dist/cjs/index.cjs, dist/esm/index.js...
(!) Unresolved dependencies
https://rollupjs.org/troubleshooting/#warning-treating-module-as-external-dependency
@eslint/css-tree (imported by "src/languages/css-language.js")
@eslint/plugin-kit (imported by "src/languages/css-source-code.js")
(!) Mixing named and default exports
https://rollupjs.org/configuration-options/#output-exports
The following entry modules are using named and default exports together:
src/index.js
Consumers of your bundle will have to use chunk.default to access their default export, which may not be what you want. Use `output.exports: "named"` to disable this warning.
created dist/cjs/index.cjs, dist/esm/index.js in 67ms
> @eslint/[email protected] build:dedupe-types
> node tools/dedupe-types.js dist/cjs/index.cjs dist/esm/index.js
> @eslint/[email protected] build:cts
> node -e "fs.copyFileSync('dist/esm/index.d.ts', 'dist/cjs/index.d.cts')" && node tools/update-cts.js dist/cjs/types.cts dist/cjs/index.d.cts
[@eslint/css] nr test
> @eslint/[email protected] pretest
> npm run build
> @eslint/[email protected] build
> npm run build:rules && rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && npm run build:cts
> @eslint/[email protected] build:rules
> node tools/build-rules.js
Recommended rules generated successfully.
Rules import file generated successfully.
src/index.js → dist/cjs/index.cjs, dist/esm/index.js...
(!) Unresolved dependencies
https://rollupjs.org/troubleshooting/#warning-treating-module-as-external-dependency
@eslint/css-tree (imported by "src/languages/css-language.js")
@eslint/plugin-kit (imported by "src/languages/css-source-code.js")
(!) Mixing named and default exports
https://rollupjs.org/configuration-options/#output-exports
The following entry modules are using named and default exports together:
src/index.js
Consumers of your bundle will have to use chunk.default to access their default export, which may not be what you want. Use `output.exports: "named"` to disable this warning.
created dist/cjs/index.cjs, dist/esm/index.js in 67ms
> @eslint/[email protected] build:dedupe-types
> node tools/dedupe-types.js dist/cjs/index.cjs dist/esm/index.js
> @eslint/[email protected] build:cts
> node -e "fs.copyFileSync('dist/esm/index.d.ts', 'dist/cjs/index.d.cts')" && node tools/update-cts.js dist/cjs/types.cts dist/cjs/index.d.cts
> @eslint/[email protected] test
> mocha tests/**/*.js
Exception during run: file://.../eslint/ecosystem/eslint-css/tests/plugin/eslint.test.js:11
import ESLintAPI from "eslint";
^^^^^^^^^
SyntaxError: The requested module 'eslint' does not provide an export named 'default'
at #asyncInstantiate (node:internal/modules/esm/module_job:302:21)
at async ModuleJob.run (node:internal/modules/esm/module_job:405:5)
at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:660:26)
at async formattedImport (.../eslint/ecosystem/eslint-css/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
at async exports.requireOrImport (.../eslint/ecosystem/eslint-css/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
at async exports.loadFilesAsync (.../eslint/ecosystem/eslint-css/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
at async singleRun (.../eslint/ecosystem/eslint-css/node_modules/mocha/lib/cli/run-helpers.js:162:3)
at async exports.handler (.../eslint/ecosystem/eslint-css/node_modules/mocha/lib/cli/run.js:375:5)
|
👍 I added |
|
Thanks! Details$ DEBUG=test:ecosystem npm run test:ecosystem
> [email protected] test:ecosystem
> node tools/test-ecosystem/index.mjs
Plugins to test: @eslint/css, @eslint/json, @eslint/markdown, @eslint-community/eslint-plugin-eslint-comments, @typescript-eslint/typescript-eslint, eslint-plugin-unicorn, eslint-plugin-vue
Clearing existing sandbox directory: .../eslint/ecosystem
Testing @eslint/css in .../eslint/ecosystem/eslint-css
[@eslint/css] git clone https://github.com/eslint/css .../eslint/ecosystem/eslint-css --depth 1
Cloning into '.../eslint/ecosystem/eslint-css'...
remote: Enumerating objects: 117, done.
remote: Counting objects: 100% (117/117), done.
remote: Compressing objects: 100% (109/109), done.
remote: Total 117 (delta 10), reused 49 (delta 2), pack-reused 0 (from 0)
Receiving objects: 100% (117/117), 117.83 KiB | 4.06 MiB/s, done.
Resolving deltas: 100% (10/10), done.
[@eslint/css] git fetch origin 26b902c4f42cccb33d7f8119a3376773e0ad91bd
remote: Enumerating objects: 1083, done.
remote: Counting objects: 100% (1083/1083), done.
remote: Compressing objects: 100% (340/340), done.
remote: Total 1007 (delta 689), reused 929 (delta 613), pack-reused 0 (from 0)
Receiving objects: 100% (1007/1007), 237.74 KiB | 6.42 MiB/s, done.
Resolving deltas: 100% (689/689), completed with 56 local objects.
From https://github.com/eslint/css
* branch 26b902c4f42cccb33d7f8119a3376773e0ad91bd -> FETCH_HEAD
[@eslint/css] git checkout 26b902c4f42cccb33d7f8119a3376773e0ad91bd
Note: switching to '26b902c4f42cccb33d7f8119a3376773e0ad91bd'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 26b902c feat: add @namespace validation to no-invalid-at-rule-placement rule (#183)
[@eslint/css] pwd
.../eslint/ecosystem/eslint-css
[@eslint/css] ni
[@eslint/css] npm link eslint
npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
added 405 packages, and audited 407 packages in 2s
144 packages are looking for funding
run `npm fund` for details
3 high severity vulnerabilities
Some issues need review, and may require choosing
a different dependency.
Run `npm audit` for details.
[@eslint/css] nr build
? Choose the agent › - Use arrow-keys. Return to submit.
❯ npm
yarn
pnpm
bun
deno
How is it for you @lumirlumir? |
|
Thanks! I've also checked, and it seems the same problem mentioned by @fasttime is occurring:
This time, running |
|
Ok great! I added |
|
Hm, now it seems that Details$ DEBUG=test:ecosystem npm run test:ecosystem -- --plugin @eslint/css
> [email protected] test:ecosystem
> node tools/test-ecosystem/index.mjs --plugin @eslint/css
Plugins to test: @eslint/css
Clearing existing sandbox directory: .../eslint/ecosystem
Testing @eslint/css in .../eslint/ecosystem/eslint-css
[@eslint/css] git clone https://github.com/eslint/css .../eslint/ecosystem/eslint-css --depth 1
Cloning into '.../eslint/ecosystem/eslint-css'...
remote: Enumerating objects: 117, done.
remote: Counting objects: 100% (117/117), done.
remote: Compressing objects: 100% (109/109), done.
remote: Total 117 (delta 10), reused 49 (delta 2), pack-reused 0 (from 0)
Receiving objects: 100% (117/117), 117.83 KiB | 697.00 KiB/s, done.
Resolving deltas: 100% (10/10), done.
[@eslint/css] git fetch origin 26b902c4f42cccb33d7f8119a3376773e0ad91bd
remote: Enumerating objects: 1083, done.
remote: Counting objects: 100% (1083/1083), done.
remote: Compressing objects: 100% (340/340), done.
remote: Total 1007 (delta 689), reused 929 (delta 613), pack-reused 0 (from 0)
Receiving objects: 100% (1007/1007), 237.74 KiB | 1.72 MiB/s, done.
Resolving deltas: 100% (689/689), completed with 56 local objects.
From https://github.com/eslint/css
* branch 26b902c4f42cccb33d7f8119a3376773e0ad91bd -> FETCH_HEAD
[@eslint/css] git checkout 26b902c4f42cccb33d7f8119a3376773e0ad91bd
Note: switching to '26b902c4f42cccb33d7f8119a3376773e0ad91bd'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 26b902c feat: add @namespace validation to no-invalid-at-rule-placement rule (#183)
[@eslint/css] pwd
.../eslint/ecosystem/eslint-css
[@eslint/css] ni
[@eslint/css] npm link eslint
npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported
added 405 packages, and audited 407 packages in 18s
144 packages are looking for funding
run `npm fund` for details
3 high severity vulnerabilities
Some issues need review, and may require choosing
a different dependency.
Run `npm audit` for details.
[@eslint/css] nr build
[@eslint/css] nr test
All tests completed successfully. |
|
Well, I'm stumped. @fasttime do you have bandwidth to "self-service" this? Try out combinations of the |
|
There are only 7 repos, can't we just add different commands to |
|
FYI, the lastest change in It will be much easier in this way. |
I also think it's a good idea to add commands for each repo explicitly 👍 |
|
All right. I really love the idea of using an automatic system but I see the folly of trying to get that to work 🥲. Switched over to manual hardcoded commands. |
| return spawn.sync(command, args, { | ||
| cwd: directory, | ||
| stdio: log.enabled ? "inherit" : undefined, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next issue I'm seeing is that the ecosystem test passes even when an individual package test fails. For example npm install in @eslint/markdown is currently failing due to a build error:
[@eslint/markdown] npm install
> @eslint/[email protected] prepare
> npm run build
> @eslint/[email protected] build
> npm run build:rules && tsc && npm run build:update-rules-docs
> @eslint/[email protected] build:rules
> node tools/build-rules.js
Recommended rules generated successfully.
Rules import file generated successfully.
src/index.js:67:2 - error TS2742: The inferred type of 'configs' cannot be named without a reference to '../node_modules/eslint/node_modules/@eslint/core/dist/cjs/types.cjs'. This is likely not portable. A type annotation is necessary.
67 configs: {
~~~~~~~~~~
68 "recommended-legacy": {
~~~~~~~~~~~~~~~~~~~~~~~~~
...
129 ],
~~~~
130 },
~~
Found 1 error in src/index.js:67It looks like the ecosystem test isn't failing when the underlying command fails. We should probably check the result of spawn.sync() to determine if the command has failed. The status property should contain the exit code of the command, if the command was run.




Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain: internal CI change; implements eslint/rfcs#127
What changes did you make? (Give an overview)
Fixes #19139.
Adds an initial
.github/workflows/ecosystem.ymland package script.Is there anything you'd like reviewers to focus on?