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

Use Grunt's file resolution #122

Conversation

jpommerening
Copy link
Contributor

grunt-karma should leave the file globbing to Grunt and it should support Karma's file options for watching files, including them, etc..

The former was first suggested in #60, but did not make it into master. The latter, well there is #51, but tl;dr and oh god, it's full of diffs ✨.

I implemented this by appending the files that are specified "the Grunt way" (this.files[0..n].src) to the files specified via options.files.

For each object {src: [...]} in this.files[0..n] the code generates an array containing file objects, that may optionally have included, watched and/or served set and finally concats/flattens them to options.files (here):

options: {
  files: ['index.js']
},
target: {
  files: [
    { src: 'test/**/*.js' },
    { src: 'lib/**/*.js', included: false }
  ]
}
/* becomes */
files = [
  'index.js',
  { pattern: 'test/file1.js' },
  { pattern: 'test/file2.js' },
  // ...
  { pattern: 'lib/file1.js', included: false },
  { pattern: 'lib/file2.js', included: false },
  // ...
];

@dignifiedquire
Copy link
Member

@jpommerening thanks a lot, looking over this so far I like it. Two things

@jpommerening
Copy link
Contributor Author

Can you please squash the commits and change the commit message based on http://karma-runner.github.io/0.12/dev/git-commit-msg.html

Sure!

Is this a backwards compatible change or not?

Phew. I'd say it depends.
On one hand, all the examples that are given in the README.md still work, because they put the files array inside the task options.
On the other hand, have a look at the following example:

karma: {
   options: { /* stuff */ },
   unit: {
      files: [ 'a.js', 'b.js' ]
   }
}

Without this PR merged that could work, because the task is mixing this.options() and this.data() into one big object. (Though it was already suggested in #21 to wrap the files list in options…)
After merging the PR this breaks, because Grunt does not understand that kind of syntax (yet?).

Possible fix:

karma: {
   options: { /* stuff */ },
   unit: {
      src: [ 'a.js', 'b.js' ]
   }
}

@dignifiedquire
Copy link
Member

Thanks for the quick answer, I think as long as all the documented ways still work, it's safe to do this and long overdue anyway.

Use the task's `files` property, but merge with `options.files`,
so we can specify some files that should be available in all test-targets.
Grunt's file objects may be extended with the file options supported
by Karma (included, watched).
Projects that use the undocumented `karma.<target>.files` option should
rename this option and use `karma.<target>.src` instead.
@dignifiedquire
Copy link
Member

Thanks a lot, and sorry for the delay. Merged as 6881024

@jpommerening
Copy link
Contributor Author

Awesome, thanks! :)

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

Successfully merging this pull request may close these issues.

2 participants