New -default-concurrency parameter to junit-runner

Review Request #3753 — Created April 24, 2016 and submitted

Eric Ayers
pants
zundel/junit-concurrency-param
3191, 3265
3707
699972d...
pants-reviews
benjyw, gmalmquist, jsirois, mateor, stuhood

Followon to https://rbcommons.com/s/twitter/r/3707/ this creates a new command line parameter for the JUnit runner that will allow us to more precisely specify the type of concurrency desired.

Updated ConsoleRunnerTest and added two new test cases to show serial behavior for classes not annotated with @TestSerial

CI green at https://travis-ci.org/pantsbuild/pants/builds/125445433

John Sirois
  1. LGTM, all debatable small stuff.
  2. It would be a bit more DRY to give the enum behavior and have each instance answer shouldRunClassParallel and shouldRunMethodParallel.

    1. Thanks for the suggestion.

  3. I feel better about fail-fast here now that the booleans are gone, Preconditions.checkNotNull

  4. Well, I guess there was precedent for not doing null checks, but if you're amenable, here and for defaultConcurrency.

  5. Kill debug output?  You may intend for this to live - I don't like any test to log past development iteration, but we may reasonably disagree about this.
    1. nope, it is redundant here as the TestRegistry... call above also creates output. I have another outstanding PR that I copied this from, I'll go back and update it.

  6. 
      
Eric Ayers
John Sirois
  1. 
      
  2. Kill an extra space: s/parallelize  /parallelize /
  3. Inline one way or the other.
  4. 
      
Eric Ayers
Garrett Malmquist
  1. 
      
  2. Nice. Maybe javadoc? Though it is pretty obvious what this is supposed to do.

  3. 
      
Chris Heisterkamp
  1. 
      
  2. Is this import used?
  3. 
      
Chris Heisterkamp
  1. Ship It!
  2. 
      
Mateo Rodriguez
  1. Ship It!
  2. 
      
Eric Ayers
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks John, Garrett, Chris and Mateo. In master at 94388b3

Loading...