jasmine-contrib / grunt-jasmine-node

Grunt task for running jasmine-node
MIT License
67 stars 99 forks source link

Added support for jasmine-node 2.0.0 #41

Closed fiznool closed 9 years ago

fiznool commented 10 years ago

Although jasmine-node 2.0.0 is still in beta, I've found it stable enough to use. This PR brings grunt-jasmine-node up to Jasmine 2.0, as requested in #38.

tomyam1 commented 10 years ago

I think you need to add extensions: options.extensions to jasmineOptions, otherwise options.extensions would be undefined in jasmine.run.

Other than that, this works fine for me.

fiznool commented 10 years ago

options.extensions is added to the options.match regex on line 47 - does it still need to be passed through to jasmine-node otherwise?

tomyam1 commented 10 years ago

This PC removed the loading of helpers.

-      if (options.useHelpers) {
 -        this.filesSrc.forEach(function(path){
 -          jasmine.loadHelpersInFolder(path, new RegExp(options.helperNameMatcher + "?\\.(" + options.extensions + ")$", 'i'));
 -        });
 -      }

The loading of helpers in jasmine-node needs options.extensions https://github.com/mhevery/jasmine-node/blob/Jasmine2.0/src/runner.coffee

runSpecs = (config) ->
...
for specFolder in options.specFolders
        jasmine.loadHelpersInFolder specFolder,
                                    new RegExp("helpers?\\.(#{options.extensions})$", 'i')

But still, this PC would change grunt-jasmine-node because before it allowed to set options.helperNameMatcher and jasmine-node have it constant at helpers (from the regex above).

Maybe the following would work:

jasmineLoader = require('jasmine-node/jasmine-loader')
jasmineLoader.loadHelpersInFolder(...)
fiznool commented 10 years ago

My mistake, I can see the issue.

I actually think there is quite a bit of redundant processing in the grunt task. IMO it needs to be rewritten to just delegate work to jasmine-node, rather than trying to do processing itself. For example, the matcher logic is in both jasmine-node and grunt-jasmine-node.

When I get some time I will attempt a rewrite to perform more delegation.

fiznool commented 10 years ago

I've taken a stab at refactoring. The PR should now hand off the options to jasmine-node wherever possible. There is no longer a separation between specs and helpers (see below) so the specs should all be referenced in the src or all option, and it also includes a fix for the grunt verbose option as discussed in #43 and #44.

@tepez as of Jasmine 2.0, helpers and specs are all the same thing. You add custom matchers in a global beforeEach() block, and it is run through the jasmine runner just as a spec is. So there is no longer any need to differentiate between a helper and a spec.

What it does mean, is that the helper is going to need to have spec somewhere in the filename by default, as this is the regex pattern that jasmine-node uses to load files into the jasmine runner. This can be altered via the grunt options if needs be.

easternbloc commented 10 years ago

+1

tomyam1 commented 10 years ago

Looks good. Will try it soon

fiznool commented 10 years ago

Any further update on this? Would be great to point my app to this package instead of my branch.

tomyam1 commented 10 years ago

@fiznool sorry for the late response. I used your fork today with success. I had to change some of the settings because now that settings are handled by jasmine-node itself, some settings are handled differently.

fiznool commented 10 years ago

@tepez OK great. Any idea on when this will be pulled in?

tomyam1 commented 10 years ago

asap I hope I think this is a question for @s9tpepper or @creynders

s9tpepper commented 10 years ago

I'm gonna look at this tonight, sorry guys been juggling a few things last couple of weeks.

volkhin commented 10 years ago

Any update on this?

fiznool commented 10 years ago

@volkhin the maintainers of this repo seem to be very slow to merge in PRs. In the meantime, you can point to my fork directly in your package.json file as follows:

{
  "devDependencies": {
    "grunt-jasmine-node": "fiznool/grunt-jasmine-node"
  }
}
volkhin commented 10 years ago

@fiznool yeah, did it with my own fork. But looks like this is a problem for many grunt plugins. Switched to gulp already:)

chesleybrown commented 10 years ago

Hey @fiznool. You should increment and tag your release so we don't have to directly reference master on your repo.

For example:

{
  "devDependencies": {
    "grunt-jasmine-node": "fiznool/grunt-jasmine-node#0.4.0"
  }
}
fiznool commented 10 years ago

Thanks @chesleybrown this is now done. A slight change from your suggested tag, as per Github's guidelines the tag is prefixed with a v:

{
  "devDependencies": {
    "grunt-jasmine-node": "fiznool/grunt-jasmine-node#v0.4.0"
  }
}
chesleybrown commented 10 years ago

Perfect. Thanks!

henrytseng commented 10 years ago

+1

henrytseng commented 10 years ago

@fiznool I noticed that the reports aren't being generated?

fiznool commented 10 years ago

@henrytseng sorry for the late reply. I've never used reports so I'm unsure how they work, however all this task does is delegate to jasmine-node 2.0, which doesn't seem to support some of the reports?

fiznool commented 10 years ago

I've just pushed an update to my branch to reference the latest jasmine-node 2.0 branch, 2.0.0-beta4. @henrytseng this might restore the reporters?

hutson commented 10 years ago

+1

fiznool commented 9 years ago

For anybody else waiting for Jasmine 2.0 support, I'd suggest migrating to this plugin instead: https://github.com/onury/grunt-jasmine-nodejs