Maven Surefire

Support junit core for parallel running of tests

Details

  • Type: New Feature New Feature
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 2.5
  • Component/s: Junit 4.x support
  • Labels:
    None
  • Environment:
    All
  • Complexity:
    Intermediate
  • Testcase included:
    yes
  • Patch Submitted:
    Yes
  • Number of attachments :
    4

Description

The enclosed patch adds junitcore support to surefire. The patch requires junit 4.6 (the latest released version) to compile, but is only activated when running with the 4.7 snapshot or higher (due to some bugs in 4.6). The patch adds one extra setting to the surefire plugin. More details at http://incodewetrustinc.blogspot.com/

The new plugin also requires an external library which can be found at http://github.com/krosenvold/configurable-parallel-computer/tree/master, which will be bumped to 1.0 when/if you decide to accept the patch. I am requesting that the junit project actually accept the features of the configurable-parallel-computer as a standard feature in junit 4.7, but that's not decided yet. I do not have a public maven repo that is hosting configurable-parallel-computer, but was hoping maybe you could publish it ?

  1. surefire.patch
    02/Jul/09 7:36 AM
    49 kB
    Kristian Rosenvold
  2. surefirev2.patch
    07/Oct/09 3:25 PM
    112 kB
    Kristian Rosenvold
  3. surefirev3.patch
    04/Nov/09 4:42 AM
    69 kB
    Kristian Rosenvold
  4. surefirev4.patch
    04/Nov/09 10:10 AM
    72 kB
    Kristian Rosenvold

Issue Links

Activity

Hide
Kristian Rosenvold added a comment -

Attached is an updated patch for junit 4.7. The revised patch also has no dependencies to any external libraries.

Show
Kristian Rosenvold added a comment - Attached is an updated patch for junit 4.7. The revised patch also has no dependencies to any external libraries.
Hide
Brett Porter added a comment -

Who are the original authors in the dual licensed pieces of code and where did it come from?

Show
Brett Porter added a comment - Who are the original authors in the dual licensed pieces of code and where did it come from?
Hide
Kristian Rosenvold added a comment - - edited

I am. I made it dual licensed to be compatible with the junit license. If this is a problem I can change it - I hope The project is located at http://github.com/krosenvold/configurable-parallel-computer

Show
Kristian Rosenvold added a comment - - edited I am. I made it dual licensed to be compatible with the junit license. If this is a problem I can change it - I hope The project is located at http://github.com/krosenvold/configurable-parallel-computer
Hide
Brett Porter added a comment -

It doesn't have to be, but ideally it would all have the "licensed to Apache..." header they all share. Since it is your own work you can certainly do that. Would you mind resubmitting?

I would like some other changes and clarification:

  • instead of "junitcore" is this basically a "junit 4.7+" feature? I got a bit confused by the terminology
  • the version parsing seems unusual - can we use a range instead ("[4.7,)")
  • can you remove the .orig files from the patch?
  • what is the purpose of the separated directory scanner?

Let me know if you need any more info on these.

Show
Brett Porter added a comment - It doesn't have to be, but ideally it would all have the "licensed to Apache..." header they all share. Since it is your own work you can certainly do that. Would you mind resubmitting? I would like some other changes and clarification:
  • instead of "junitcore" is this basically a "junit 4.7+" feature? I got a bit confused by the terminology
  • the version parsing seems unusual - can we use a range instead ("[4.7,)")
  • can you remove the .orig files from the patch?
  • what is the purpose of the separated directory scanner?
Let me know if you need any more info on these.
Hide
Kristian Rosenvold added a comment -

Yes to all. Will resubmit tomorrow.

The purpose of the separated directory scanner was really just to get it unit-testable as an individual piece. I suppose I couldn't resist the urge to make things more unit-testable, since they're both faster and easier to write. There is also a unit-test for that separate piece of code. At that point I guess It was a bit irresponsible of me not to replace usage in the other plugins too, but I suppose I realized I was getting a bit carried away. The api-contract of the separated scanner is also slightly cleaner than the original.

I can take this both ways (revert to old-style or change the other plugins), what do you think ?

Show
Kristian Rosenvold added a comment - Yes to all. Will resubmit tomorrow. The purpose of the separated directory scanner was really just to get it unit-testable as an individual piece. I suppose I couldn't resist the urge to make things more unit-testable, since they're both faster and easier to write. There is also a unit-test for that separate piece of code. At that point I guess It was a bit irresponsible of me not to replace usage in the other plugins too, but I suppose I realized I was getting a bit carried away. The api-contract of the separated scanner is also slightly cleaner than the original. I can take this both ways (revert to old-style or change the other plugins), what do you think ?
Hide
Brett Porter added a comment -

that's fine, I couldn't see a big change so just checking whether it was meant to do something I couldn't see

Show
Brett Porter added a comment - that's fine, I couldn't see a big change so just checking whether it was meant to do something I couldn't see
Hide
Kristian Rosenvold added a comment -

This is version 3 of the patch (surefirev3.patch). Includes all requested changes.

I will try to submit a subsequent patch to change the other providers to use the external scanner, but this patch is totally self-contained.

Show
Kristian Rosenvold added a comment - This is version 3 of the patch (surefirev3.patch). Includes all requested changes. I will try to submit a subsequent patch to change the other providers to use the external scanner, but this patch is totally self-contained.
Hide
Kristian Rosenvold added a comment -

It turns out I had done "the right" thing and extracted all the directory scanning to the separate class, so this is effectively in use by all the providers (it was done in the base class for the providers and I extracted it to get more precise test coverage and re-use for the junit 4.7 provider)

Since I'm not totally fluent in the maven internals, you may also want to just check the methods isJunit40to46 and isJunit47Compatible in the revised patch, just in case there's a better way to do it.

Show
Kristian Rosenvold added a comment - It turns out I had done "the right" thing and extracted all the directory scanning to the separate class, so this is effectively in use by all the providers (it was done in the base class for the providers and I extracted it to get more precise test coverage and re-use for the junit 4.7 provider) Since I'm not totally fluent in the maven internals, you may also want to just check the methods isJunit40to46 and isJunit47Compatible in the revised patch, just in case there's a better way to do it.
Hide
Kristian Rosenvold added a comment - - edited

v4 also includes what I hope is the appropriate license headers in all files.

Also note that after I artifactid & name of the provider to surefire-junit47 I had to run mvn -N install within the surefire-providers folder and the root folder to get things to compile (assuming you have compiled the previous version within the same checkout)

Show
Kristian Rosenvold added a comment - - edited v4 also includes what I hope is the appropriate license headers in all files. Also note that after I artifactid & name of the provider to surefire-junit47 I had to run mvn -N install within the surefire-providers folder and the root folder to get things to compile (assuming you have compiled the previous version within the same checkout)
Hide
Igor Minar added a comment -

hi there,

Will the patch be accepted? This new feature looks very interesting.

thanks,
Igor

Show
Igor Minar added a comment - hi there, Will the patch be accepted? This new feature looks very interesting. thanks, Igor
Hide
Daniel Gredler added a comment -

Please apply this patch (or similar) and let me run my unit tests in half the time

Show
Daniel Gredler added a comment - Please apply this patch (or similar) and let me run my unit tests in half the time
Hide
Stephen Connolly added a comment -

targetting for 2.5

Show
Stephen Connolly added a comment - targetting for 2.5
Hide
Stephen Connolly added a comment -

r895651.

Show
Stephen Connolly added a comment - r895651.
Hide
Paul Benedict added a comment -

This issue was created when JUnit 4.7 was current. The latest version is now 4.8.1.

Show
Paul Benedict added a comment - This issue was created when JUnit 4.7 was current. The latest version is now 4.8.1.
Hide
Kristian Rosenvold added a comment -

The patch I supplied uses "any" version of surefire >= 4.7. 4.8 is also recommended, since it contains 2 concurrency patches I submitted to junit. Hopefully, 4.8.1 will find its way to a maven repo soon.

Show
Kristian Rosenvold added a comment - The patch I supplied uses "any" version of surefire >= 4.7. 4.8 is also recommended, since it contains 2 concurrency patches I submitted to junit. Hopefully, 4.8.1 will find its way to a maven repo soon.

People

Vote (12)
Watch (11)

Dates

  • Created:
    Updated:
    Resolved: