Skip to content
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

chore: fix failing tests #163

Closed
wants to merge 1 commit into from
Closed

chore: fix failing tests #163

wants to merge 1 commit into from

Conversation

Trott
Copy link

@Trott Trott commented Nov 3, 2020

Two tests are written that require ESLint@7, but the project specifies
ESLint@6. Move to ESLint@7.

BREAKING CHANGE: ESLint@7 drops support for Node.js 8.x.

@eslint-deprecated
Copy link
Contributor

Hi @Trott!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The commit message tag must be one of the following:

    The Tag is one of the following:

    • Fix - for a bug fix.
    • Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
    • New - implements a new feature.
    • Breaking - for a backwards-incompatible enhancement or feature.
    • Docs - changes to documentation only.
    • Build - changes to build process only.
    • Upgrade - for a dependency upgrade.
    • Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).

    You can use the labels of the issue you are working on to determine the best tag.

  • There should be a space following the initial tag and colon, for example 'New: Message'.

  • The first letter of the tag should be in uppercase

Read more about contributing to ESLint here

@Trott Trott force-pushed the fix-tests branch 7 times, most recently from bc59796 to 1850b90 Compare November 3, 2020 05:23
Two tests are written that require ESLint@7, but the project specifies
ESLint@6. Require ESLint@7 in devDependencies.

BREAKING CHANGE: ESLint@7 does not support Node.js 8.x.
@Trott
Copy link
Author

Trott commented Nov 7, 2020

(Fixed the issues flagged by the bot and force pushed. This is ready to be reviewed.)

@btmills
Copy link
Member

btmills commented Nov 9, 2020

Hi @Trott, thanks for pointing this out! This plugin is compatible with the ESLint v6 processor API, and most of the unit tests use that. The examples use the friendlier ESLint v7 processor API. As a way to verify both work, we run the examples as an additional test step. CI runs the install-examples npm script before npm test so that the examples have ESLint v7 installed. However, I see npm run install-examples is missing from the contributing docs, so git clone ...; cd ...; npm install; npm test will fail. Any ideas?

Ideally there'd be a way to automatically run npm install for the examples as part of a local npm install but not when the plugin is installed as a dependency. Then the separate install-examples step wouldn't be necessary. Maybe we could abuse the prepare script for that? Worst case we can add npm run install-examples to the contribution section at the bottom of the readme, but something automatic would be much better.

@Trott Trott closed this Nov 9, 2020
btmills added a commit that referenced this pull request Nov 15, 2020
Previously, CI was explicitly running the `install-examples` script as
its own step. This was also necessary but not documented locally, so
`git clone; npm install; npm test` would show a couple failures when the
examples inherited the wrong ESLint dependency. The examples'
dependencies should now be installed automatically when running `npm
install` locally without any arguments.

Originally reported in #163.
btmills added a commit that referenced this pull request Nov 15, 2020
Previously, CI was explicitly running the `install-examples` script as
its own step. This was also necessary but not documented locally, so
`git clone; npm install; npm test` would show a couple failures when the
examples inherited the wrong ESLint dependency. The examples'
dependencies should now be installed automatically when running `npm
install` locally without any arguments.

Originally reported in #163.
btmills added a commit that referenced this pull request Nov 15, 2020
Previously, CI was explicitly running the `install-examples` script as
its own step. This was also necessary but not documented locally, so
`git clone; npm install; npm test` would show a couple failures when the
examples inherited the wrong ESLint dependency. The examples'
dependencies should now be installed automatically when running `npm
install` locally without any arguments.

Originally reported in #163.
btmills added a commit that referenced this pull request Nov 15, 2020
Previously, CI was explicitly running the `install-examples` script as
its own step. This was also necessary but not documented locally, so
`git clone; npm install; npm test` would show a couple failures when the
examples inherited the wrong ESLint dependency. The examples'
dependencies should now be installed automatically when running `npm
install` locally without any arguments.

Originally reported in #163.
btmills added a commit that referenced this pull request Nov 26, 2020
Previously, CI was explicitly running the `install-examples` script as
its own step. This was also necessary but not documented locally, so
`git clone; npm install; npm test` would show a couple failures when the
examples inherited the wrong ESLint dependency. The examples'
dependencies should now be installed automatically when running `npm
install` locally without any arguments.

Originally reported in #163.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants