Change test extension to .lint-test.js

[mitchlloyd]

This addresses ember-cli/ember-cli-jshint#5, and is a follow up from the discussion in #41.

To test these changes, I installed a new Ember CLI app, and created npm links in locally cloned repos:

ember-cli-app => ember-cli-jshint => broccoli-jshint

Now the files generated end in .lint-test.js, aligning with ember-cli-qunit:

Running all tests show the linting tests:

Using the "Disable Linting" checkbox works:

@Turbo87 could you double check that this fixes the issue? Testing this is a little involved so I would love to get a second set of eyes.

[mitchlloyd]

I'm not sure why this error message changed, but I figure it must be a bug fix in jshint. The new error message (col 10) is better than the old error message (col 1).

[rwjblue]

Looks good to me. Will land once @Turbo87 has a chance to review.

[Turbo87]

Afk already, I'll test this out tomorrow. One question though, what happens when eslint and jshint and JSCS all create a lint-test file? Won't that create conflicts?

[rwjblue]

One would override the other, but it seems like an edge case that is not terribly common.

One potential solution to that would be to use jshint.lint-test.js (and ember-cli-template-lint would use template.lint-test.js), but again I'm not certain it really is a big deal...

[Turbo87]

The lint-test extension is for the QUnit checkbox to disable the lint tests, correct?

[rwjblue]

Correct

[mitchlloyd]

@Turbo87 The naming conflict issue is something I hadn't considered. The lint-test extension is supposed to allow ember-cli-qunit to recognize any library's files as tests, and as linting tests so that they are automatically run and can be disabled with that checkbox.

So here are the constraints we're working with:

• Use the filename to convey "I am a test"
• Use the filename to convey "I am a linting test"
• Use the filename to differentiate linting tests for the same file
• Avoid clashes with user-defined tests
• Avoid library churn as much as possible.
• Don't add special code for every linting library to ember-cli-qunit

Today we have these linting extensions that I know of:

• .jshint.js - working JSHint extension that we would like to move away from
• .lint.js - current non-working JSHint extension
• .lint-test.js - working ESLint extension
• .jscs-test.js - non-disableable JSCS extension
• .template-lint-test.js - non-disableable extension from ember-cli-template-lint

Using -test (or _test) seems like a long-standing convention for identifying a test file in QUnit and other testing frameworks. Keeping this convention for all test files (including linting tests) seems like a good idea.

Requiring the . (which ember-cli-qunit did for .jshint.js linting tests) seems good as well to avoid naming conflicts with any user test files that might end in lint-test. All of these libraries include a . in their special naming portion of the file.

As @rwjblue pointed out jshint.lint-test.js is an option that would work with the current version of ember-cli-qunit.

Another option would be to expand the linting check in ember-cli-qunit to allow .jshint-lint-test, meaning that anything that matches the pattern .<name>-lint-test would work as well as lint-test. The advantage of this is that would immediately make the .template-lint-test extension work.

Given the current state of the libraries I listed I feel like the simplicity of <namespace>.lint-test.js seems reasonable. That would mean the future would have:

• .jshint.lint-test - No matter what we have to fix the current .lint.js issue
• .eslint.lint-test.js - Don't need to change this now
• .jscs.lint-test.js - Would enhance to use the "Disable Linting" button.
• .ember-template-lint.lint-test.js - Would enhance ember-template-lint to use "Disable Linting button"

I like that the specificity in the filename decreases from left to right user-name.library-name.type-of-test.type-of-file, and that this works with the current-version of ember-cli-qunit.

Another option could be to allow each of these ember-cli-linting libraries to register their specific extension as a type of linting test.

[Turbo87]

I tend to agree with @mitchlloyd. Having ESLint and JSHint run in parallel is most likely an edge case but something we should consider, particularly in the transition period between the two.

Since broccoli-jshint is a generic lib that is not directly related to the Ember CLI ecosystem (and the linting checkbox in QUnit) I'm wondering if we should just set the targetExtension in ember-cli-jshint and be done. That way we won't have the need for another major release here just to change the extension again.

[rwjblue]

Version numbers are not costly 😺

[mitchlloyd]

Yeah if we can handle this in the integration libraries between ember-cli and the broccoli libs that seems better. Closing this for now.