karma-runner / grunt-karma

Grunt plugin for Karma.
MIT License
468 stars 116 forks source link

Use Grunt's file resolution #122

Closed jpommerening closed 9 years ago

jpommerening commented 10 years ago

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 :sparkles:.

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 commented 10 years ago

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

jpommerening commented 10 years ago

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 commented 10 years ago

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.

dignifiedquire commented 9 years ago

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

jpommerening commented 9 years ago

Awesome, thanks! :)