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

Only fails on modified files #84

Closed
alexlafroscia opened this issue Jul 1, 2016 · 8 comments
Closed

Only fails on modified files #84

alexlafroscia opened this issue Jul 1, 2016 · 8 comments

Comments

@alexlafroscia
Copy link
Collaborator

I'm seeing some pretty weird behavior, and I'm not sure where in the pipeline the issue is.

I've been working on porting Ember Suave's rules over to ESLint, and so have installed ember-cli-eslint on a fresh Ember app as well as my ESLint plugin. After running ember test it shows that all of the tests, even when the tests should fail.

If I go into one of the files that should fail, app/app.js for example, and modify it in any way, the test now fails the way I expect it to. It's like the passing test is being cached for some reason. I've trying nuking the dist and tmp directories, but that didn't help at all. Is there somewhere else where these results might be cached?

I do know that ESLint can cache results which sounds like the behavior that I'm experiencing, but I can't find a .eslintcache directory anywhere.

@BrianSipple
Copy link
Collaborator

Interesting.

I tried replicating this by generating a new app and writing a rule that would cause app/app.js to fail (prefer-const: 2). I played with moving this rule between .eslintignore and tests/.eslintignore, wondering if they way both files are read during tests would cause the behavior, but that didn't seem to do it.

How are you implementing your own rules on top of those generated in .eslintignore and tests/.eslintignore on a fresh ember install ember-cli-eslint?

@alexlafroscia
Copy link
Collaborator Author

alexlafroscia commented Jul 2, 2016

I've implemented a few rules in a plugin that you can fine here.

This is my current configuration:

module.exports = {
  extends: './node_modules/ember-cli-eslint/coding-standard/ember-application.js',
  plugins: [
    'ember-suave'
  ],
  rules: {
    // 'prefer-const': ['error'],

    // Ember Suave rules
    // https://github.com/alexlafroscia/eslint-plugin-ember-suave
    'ember-suave/no-const-outside-module-scope': ['error'],
    'ember-suave/no-direct-property-access': ['error'],
    'ember-suave/require-access-in-comments': ['error'],
    'ember-suave/require-const-for-ember-properties': ['error']
  }
};

If I run ember test with the above configuration, all of the linting passes. However, the no-direct-property-access rule should be failing due to Ember.Application being access directly in app/app.js.

If I update the config to look like this:

module.exports = {
  extends: './node_modules/ember-cli-eslint/coding-standard/ember-application.js',
  plugins: [
    'ember-suave'
  ],
  rules: {
    'prefer-const': ['error'],

    // Ember Suave rules
    // https://github.com/alexlafroscia/eslint-plugin-ember-suave
    'ember-suave/no-const-outside-module-scope': ['error'],
    'ember-suave/no-direct-property-access': ['error'],
    'ember-suave/require-access-in-comments': ['error'],
    'ember-suave/require-const-for-ember-properties': ['error']
  }
};

I now get one failing with ember test, and two linter errors on the same file; one for prefer-const and one for the Ember.Application access. No changes other than enabling the one prefer-const rule.

Interesting, the router.js file should also be failing (for the Ember.Router access) but that isn't showing up either.


Also, if I disable the prefer-const rule again, and change app/app.js to look like this:

// foo

import Ember from 'ember';
import Resolver from './resolver';
import loadInitializers from 'ember-load-initializers';
import config from './config/environment';

let App;

Ember.MODEL_FACTORY_INJECTIONS = true;

App = Ember.Application.extend({
  modulePrefix: config.modulePrefix,
  podModulePrefix: config.podModulePrefix,
  Resolver
});

loadInitializers(App, config.modulePrefix);

export default App;

now ember test has one failure, for the Ember.Application access in that file. All I had to do was alter the file to include // foo at the top which is why it seems to me like the results of the original app/app.js file are being cached somewhere. Adding that comment on the first line seems like a bizarre thing to affect the linting results.

@alexlafroscia
Copy link
Collaborator Author

I'm trying to reproduce in a new repo, but am having trouble. I'm going to play with it and see if I can pin down what exact steps cause it to have the above behavior.

@alexlafroscia
Copy link
Collaborator Author

Ehh... I've tried reproducing it by re-tracing my steps but just can't get it to have the same problem on a fresh repo, even though this particular app is still seeing the same problem. If I ever figure it out, I'll make sure to note the fix here but in the mean time don't want to waste anyone's time.

@michalsnik
Copy link
Member

michalsnik commented Sep 2, 2016

Hi @alexlafroscia, I just encountered the same issue in my project.

I wrote eslint plugin some time ago (https://github.com/netguru/eslint-plugin-netguru-ember) with rules according to our ember styleguide. I was using it without problems so far, but today I had problem with one rule, that was not passing, so I updated this plugin and released new version - which has the rule fixed.

After updating package in project eslint still shows the same error, however if I change anything in my component's file everything passes.. Looks definitely like a caching issue - the rules code isn't even executed when file not changed.

Do you know already how could I clean this cache? Thanks in advice! :)

@alexlafroscia
Copy link
Collaborator Author

alexlafroscia commented Sep 2, 2016

Now that I understand the Broccoli process a little bit more, I can probably provide a little more context. There's a cache used at the Broccoli level that makes sure that a file is not linted again if it hasn't changed. However, this caching does not take configuration changes into account, which should invalidate the entire cache. This is an optimization that should be implemented at the broccoli-lint-eslint level, most likely.

This cache, AFAIK, is persisted into a directory within $TMPDIR but I can't remember exactly which one it is. That behavior comes from broccoli-persistent-filter, if you want to dig into it more directly.

@michalsnik
Copy link
Member

Yeah, I just started digging in their repo, there is an option persist which according to their documentation should be disabled by default and clear filters state on each restart, but it doesn't.. I'll take a deeper look, maybe there is a way to overcome this :) Thanks for a fast reply, now I have better background for further investigation.

@michalsnik
Copy link
Member

Documentation:
image

vs

Code:
image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants