Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Add the tests directory to default lint include #331

Closed
edmorley opened this issue Sep 27, 2017 · 6 comments
Closed

Add the tests directory to default lint include #331

edmorley opened this issue Sep 27, 2017 · 6 comments
Assignees
Milestone

Comments

@edmorley
Copy link
Member

edmorley commented Sep 27, 2017

Currently neutrino-middleware-eslint defaults the directory include list to just [neutrino.options.source]:
https://github.com/mozilla-neutrino/neutrino-dev/blob/10819ad89d9d408f79e29487af5ea430701ac6df/packages/neutrino-middleware-eslint/index.js#L30

It seems like linting should also be run against tests - and so include default instead to [neutrino.options.source, neutrino.options.test]. (At least for neutrino lint or neutrino test, perhaps not for start/build?)

Also it looks like the include/exclude from neutrino-preset-airbnb-base can be removed, since it's just repeating what is in neutrino-middleware-eslint:
https://github.com/mozilla-neutrino/neutrino-dev/blob/10819ad89d9d408f79e29487af5ea430701ac6df/packages/neutrino-preset-airbnb-base/index.js#L24

Thoughts? :-)

@eliperelman
Copy link
Member

The reason why the test directory is currently left out is it usually requires a different set of linting rules because of the test environment, sometimes from assumed globals. My thoughts to this point have been, if they want tests linted, they can override to add the directory and make the changes to support their test environment.

If we had a good answer to this, I think I might be persuaded to switch it.

@edmorley
Copy link
Member Author

Ah fair point - happy to punt on this :-)

@constgen
Copy link
Contributor

Test middlewares can override this setting if they provide all necessary plugins for linting, are they?

@edmorley edmorley added this to the v9 milestone Apr 27, 2018
@eliperelman
Copy link
Member

eliperelman commented May 8, 2018

Is the only thing stopping us here is augmenting the eslint changes based on test middleware lint overrides?

@edmorley
Copy link
Member Author

edmorley commented May 8, 2018

I think we should do this - and just adjust the test presets to ensure they do the right thing. If there are any cases not covered by them, then it's up to the end user to either override includes or supplement eslint config globals/plugins.

@edmorley
Copy link
Member Author

edmorley commented Jun 8, 2018

All three test presets actually already configure the relevant eslint plugins/globals:
https://github.com/mozilla-neutrino/neutrino-dev/blob/05058267088b15eeff812737e81825a662b75f57/packages/jest/src/index.js#L8-L19
https://github.com/mozilla-neutrino/neutrino-dev/blob/05058267088b15eeff812737e81825a662b75f57/packages/mocha/src/index.js#L6-L10
https://github.com/mozilla-neutrino/neutrino-dev/blob/05058267088b15eeff812737e81825a662b75f57/packages/karma/index.js#L8-L12

To make things even more resilient we could always expand the "lint presets must be defined before compile presets" check to also check for the registered jest/mocha/karma commands (we'll have to hardcode their names, but guess doesn't matter too much).

@edmorley edmorley self-assigned this Jun 8, 2018
edmorley added a commit that referenced this issue Jun 14, 2018
The `tests` directory was already added to the `lint` script command
by create-project, however it was not yet listed in the `include`
for `eslint-loader`.

The testing presets already update `envs` accordingly, to prevent
errors from mocha/jest/... globals.

The redundant `eslint.extensions` option has been removed, since that
was only used by the ESLint Node API, which we no longer use.

Fixes #331.
acconrad pushed a commit to acconrad/neutrino-dev that referenced this issue Jul 13, 2018
The `tests` directory was already added to the `lint` script command
by create-project, however it was not yet listed in the `include`
for `eslint-loader`.

The testing presets already update `envs` accordingly, to prevent
errors from mocha/jest/... globals.

The redundant `eslint.extensions` option has been removed, since that
was only used by the ESLint Node API, which we no longer use.

Fixes neutrinojs#331.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants