search

karma-runner / maven-karma-plugin

Maven plugin for running tests using Karma.
Apache License 2.0
45 stars 26 forks source link

Change to the process builder to replace slashes with file.separator #22

Closed PhilHardwick closed 10 years ago

PhilHardwick commented 10 years ago

I found that when using a local install of karma (i.e. without -g in the npm install) specifying a relative path to the executable would not work across both linux and windows systems. The ability to use a local install of karma-cli in node_modules/.bin means the project can keep it's dependencies localised, removing the need for karma to be installed globally.

The extra method just does a simple replace on '/' and '\' with File.separator.

I have also added a bit to the documentation comments on the properties to make it a bit clearer what the reporting properties do.

There's a possibilty to make the default value of junitReporterFile the same as the value of junitReporter.outputFile in the karma.conf.js file but that would require a JSON library being added to extract that value. I haven't done this here but if it seems reasonable to do then I can add this in another pull request.

PhilHardwick commented 10 years ago

Yeah, you're absolutely right, I've changed that in the recent commit.

PhilHardwick commented 10 years ago

Sorry, that "closing" of the pull request was a mistake.

kelveden commented 10 years ago

Cool, looks good to me; and thanks for the PR.

I never envisaged that the plugin would be any more complicated than simply shelling out to karma hence the lack of unit tests. However, there's enough complexity been added now that it could probably do with some!

kelveden commented 10 years ago

I'll try and get a release up to maven central later in the week.

PhilHardwick commented 10 years ago

Yeah a maven central release would be really useful, thanks!

I have a few tests to go with the commit, shall I add them in a new pull request with the junit dependency in the pom?

kelveden commented 10 years ago

Released in 1.8. Sorry for the delay.

And do add the tests by all means.