[Tests]: add weekly scheduled smoke tests

[AriPerkkio]

Context: AriPerkkio/eslint-remote-tester#29

What

Adds weekly scheduled smoke tests to be run on every Sunday at 00:00. Tests can also be trigger manually (workflow_dispatch). These tests will be looking for rule crashes.

Previously these tests have been run in eslint-remote-tester's repository. So far bugs below have been found by automated CI runs:

1. jsx-no-constructed-context-values: Cannot read property 'set' of undefined jsx-no-constructed-context-values: Cannot read property 'set' of undefined #2894
2. jsx-no-constructed-context-values: Cannot read property 'type' of null jsx-no-constructed-context-values: Cannot read property 'type' of null #2895
3. no-unknown-property: allowedTags.indexOf is not a function no-unknown-property: allowedTags.indexOf is not a function #2879
4. jsx-max-depth: Maximum call stack size exceeded jsx-max-depth: Maximum call stack size exceeded #2880
5. jsx-wrap-multilines: RangeError: Invalid count value jsx-wrap-multilines: RangeError: Invalid count value #2875
6. jsx-no-script-url: Cannot read property 'type' of null jsx-no-script-url: Cannot read property 'type' of null #2871
7. no-typos: Cannot read property 'toLowerCase' of undefined no-typos: Cannot read property 'toLowerCase' of undefined #2870
8. jsx-max-depth: Cannot read property 'length' of undefined jsx-max-depth: Cannot read property 'length' of undefined #2869
9. Unreleased: Add new rule "react/no-referential-type-default-prop" [New] add no-object-type-as-default-prop rule #2848
10. forbid-dom-props forbid-dom-props: support JSXNamespacedName #2961 (comment)

How

Plugin maintainer making sure all existing rules do not crash

Utilizes https://github.com/marketplace/actions/eslint-remote-tester-runner. Each test run will check as many repositories as it can in 5 hours 30 minutes (default value of timeLimit).
There are 10K repositories in test/smoke-test-repositories.json. The first ~3300 are specifically picked for this plugin as they have /react/ in the repository name. These were collected from libraries.io.

At first the rules will be extends: ['plugin:react/all'] but it is absolutely OK to include rules: {} with settings for specific rules if wanted.

Available configuration options:

• eslint-remote-tester-run-action: https://github.com/marketplace/actions/eslint-remote-tester-runner#action-parameters
• eslint-remote-tester: https://github.com/AriPerkkio/eslint-remote-tester#configuration-options

When test fails the @github-actions bot will open new issue with title Results of weekly scheduled smoke test. It will reuse the existing issue if previous one hasn't yet been closed. It's recommended to close the issues once their findings are fixed.

🎉

[AriPerkkio]

CI's pretest steps are failing due to eslint version conflicts between dependencies.

npm ERR!   peer eslint@"^5.0.0 || ^6.0.0" from @typescript-eslint/parser@2.34.0

npm ERR! Conflicting peer dependency: eslint@7.24.0
npm ERR!   peer eslint@">=7" from eslint-remote-tester@1.1.0

The reason for eslint-remote-tester's eslint@^7 requirement is the usage of new ESLint class Nodejs API + https://eslint.org/docs/user-guide/migrating-to-7.0.0#deprecate-cliengine.

I'm not sure how to resolve this conflict here.

Merging #2825 should fix this. Or even @typescript-eslint/parser@^3.10.1 should be enough.

[AriPerkkio]

Second and last conflict: version util test was checking actual installed dependencies. Now that react is installed (required by ink) the test fails.
Fixed by [Tests]: version util test to mock "no react installed" case.

[ljharb]

this seems like it would affect our ability to test on v2 of the parser. any thoughts/concerns?

[AriPerkkio]

I've been looking at this changelog of 2.34.0 -> 3.0.0 and don't think there's anything special in v2 that is being missing after upgrading to v3. Most important thing is the node v8 support being dropped out but I think that's it. But I must say I'm not expert when it comes to AST parsers. 🙃

[ljharb]

That means we wouldn't be able to use it on node 4 and 6 tests :-/

[AriPerkkio]

That's definitely a drawback. Not sure how to resolve this. 😕

A bit off-topic but just to clarify - how was the v2.34.0 be able to run on node 4? It requires node v8 + eslint v5. ESLint v5 requires node 6. Am I reading these requirements correctly?

[ljharb]

actually you're right; the tests are set up to skip the TS tests on node < 5 (or maybe <= 5).

(this project supports down to eslint 3, which requires node 4)

[AriPerkkio]

Alright so previously with v2.34.0 the TS tests have been run for eslint@^5 + node@^8.
Now the v3 supports eslint@^5 and node@^10. The only thing that is lost is being able to test the typescript parser against node v8. ESLint version is still the same. That doesn't sound too bad, right?

[ljharb]

I think we can fix this by depending on ^2.34.0 || ^3.10.1, and in the CI config, manually installing v2 when it's node < 10.

[AriPerkkio]

Now using "@typescript-eslint/parser": "^2.34.0 || ^3.10.1" and CI seems to be passing. Does it still require changes to CI workflow configurations?

Should the version be forced here?

https://github.com/yannickcr/eslint-plugin-react/blob/f233eb77c5c55662287991a914f3a7e2c4172db0/.github/workflows/node-4%2B.yml#L65

[ljharb]

Yes, I think that's precisely the solution that's needed :-)

[AriPerkkio]

CI will now resolve version based on matrix.node: "@typescript-eslint/parser@${{ matrix.node-version >= 10 && '3' || '2' }}".
The "ternary" syntax here is a bit weird but it works.

Logs from jobs:

• latest majors (10, 7): * AFTER_INSTALL: npm install --no-save "eslint@7" "@typescript-eslint/parser@3"
• latest majors (9, 6): * AFTER_INSTALL: npm install --no-save "eslint@6" "@typescript-eslint/parser@2"

[ljharb]

hm, my other comment got lost. can the "list of repos" be fetched remotely? i don't really want to have to maintain a manual list of other people's repos if i can avoid it. (it's great to be able to override it locally if needed, ofc)

[AriPerkkio]

That's something other developers have been asking too. These repositories have been fetched with a script for libraries.io but their service timeouts often and is extremely unstable, librariesio/libraries.io#2647. Maybe I should finally build some kind of centralized solution for this.

Plugin projects could fetch these repositories from a hosted gist.github but that means they would need node-fetch or similar request library as dependency.
Maybe a better solution would be if I maintained a separate npm package which provided these repositories? Updating them would be easy, and overriding a specific repository in plugin project would be possible, e.g.

// eslint-remote-tester.config.js
const repositories = require('eslint-remote-tester-repositories');

module.exports = {
   repositories: repositories.filter(repository => repository !== 'org/some-repository-which-we-dont-want'),
   ...
}

I would like to avoid providing these from eslint-remote-tester in order to avoid publishing it often, so a separate npm package would be better.

[ljharb]

In the github action, it could be fetched with curl and then cached using the actions cache? but yes a separate npm package would also be fine.

[codecov-commenter]

Codecov Report

Merging #2963 (0894132) into master (be06fa4) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2963   +/-   ##
=======================================
  Coverage   97.38%   97.38%           
=======================================
  Files         111      111           
  Lines        7426     7426           
  Branches     2713     2713           
=======================================
  Hits         7232     7232           
  Misses        194      194           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be06fa4...0894132. Read the comment docs.

[AriPerkkio]

Now using repositories from eslint-remote-tester-repositories package.

Also a notable change - removed passing ${{ secrets.GITHUB_TOKEN }}. It is no longer required by eslint-remote-tester-run-action. It's using github.token internally which is available by default.

[ljharb]

i'm a bit confused why this is needed - npm link alone ensures that the binary is globally available

[AriPerkkio]

ESLint is unable load the plugin without this. As far as I understand, the npm link <package> creates the actual symlink under projects node_modules. Does this mean ESLint is unable to use globally available packages?

Here is example run without this line. https://github.com/AriPerkkio/eslint-plugin-react/runs/2477044926?check_suite_focus=true

Error: Configuration validation errors:
- eslintrc: Failed to load plugin 'react' declared in 'CLIOptions': Cannot find module 'eslint-plugin-react'

[ljharb]

ahhh yes, because eslint requires it.

in that case, an alternative to "link to global, then link back down" is if we add to devDeps "eslint-plugin-react": "link:./", perhaps?

[AriPerkkio]

"link:./" seems to fail but "file:./" works. No need to run link steps in workflow now. https://docs.npmjs.com/cli/v7/configuring-npm/package-json#local-paths

"eslint-plugin-react": "file:./",

[AriPerkkio]

Oh actually I'll need to revert this change. Some older node versions seem to fail. Some fail in unit-test step, some fail in installation step.

https://github.com/yannickcr/eslint-plugin-react/actions/runs/803786800

[ljharb]

seems it behaves differently in npm 6 vs 7 :-/

[ljharb]

That means we wouldn't be able to use it on node 4 and 6 tests :-/

[AriPerkkio]

Now using ljharb/actions/node/install@main and limiting workflow to yannickcr/eslint-plugin-react repository.

[AriPerkkio]

FYI, while testing this setup with latest master it seems that some unreleased changes are causing a lot of crashes for no-typos rule. Also no-unstable-nested-components crashed at least once. Could be related to #3001.

Test has been running for 35mins and found hundreds of crashes: [INFO/STATUS] Errors (617).

[ljharb]

Awesome, thanks!