Parameterized runner factory #773
Conversation
|
I think that's a better design, too! Have you considered using an additional annotation on the test class that specifies the factory instead of subclassing |
|
I did not consider an annotation, but that's a good idea. I add this feature. It will look like Do you think we should still support the constructor |
|
I added the annotation. I still would like to remove the newly added constructor |
| * runner that it should use your factory. | ||
| * | ||
| * <pre> | ||
| * @RunWith(YourParameterizedRunner.class) |
There was a problem hiding this comment.
This should reference Parameterized, right?
|
I've added a couple of comments inline. Go ahead and remove the extra constructor if you want, your call! :-) |
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| if (this == obj) |
There was a problem hiding this comment.
JUnit style is to always use braces for "if" "do" and "while" statements.
|
Applied code review except exception handling for getParametersRunnerFactory (see comment). |
| @Override | ||
| public int hashCode() { | ||
| int prime = 14747; | ||
| int result = prime + ((fName == null) ? 0 : fName.hashCode()); |
There was a problem hiding this comment.
fName can't be null, so this can be simpler
|
LGTM, but I don't use Note that github thinks this can't be merged as-is. |
|
I rebase this pull request in the evening. |
It is common to add tests to the same package as the class under test.
This class keeps the data together that are needed for creating a runner for a single data set of a parameterized test. This makes it also clear, that the computation of the name is not the responsibility of the runner but of the Parameterized class.
Separate the class from the Parameterized runner. Makes it clear, that it is designed for reuse.
…meters. There's now an explicit component for creating the runner of a single set of parameters. IMHO this is a better extension point than overriding a method. The factory can be specified by the @UseParametersRunnerFactory annotation or you can subclass the Parameterized class and provide the factory to its constructor.
|
Ready to be merged. Thanks everybody for finding the code smells. |
|
@marcphilipp are you okay merging this? |
|
Yep, go ahead! |
|
@stefanbirkner can you update the release notes? |
|
Is there an example anywhere of a custom parameters runner using this new factory? |
| * <h3>Create different runners</h3> | ||
| * <p> | ||
| * By default the {@code Parameterized} runner creates a slightly modified | ||
| * {@link BlockJUnit4ClassRunner} for each set of parameters. You can build an |
There was a problem hiding this comment.
I believe the doc should read "You can build your own" instead of "You can build an own". Correct?
|
Would it be a good idea to have @inherited on the UseParametersRunnerFactory annotation as the @RunWith annotation has @inherited on it. |
|
@vimil good catch! Would you be willing to send us a pull request? If so, please include a test case that fails currently but would pass if |
|
@kcooney, I've sent a pull request :) |
This is a follow-up to 61badf2 (Pull Request #564), that added a new method
protected Runner createRunner(String pattern, int index, Object[] parameters). This pull request provides a different way for building own runners. You have to create aParametersRunnerFactorythat creates the runners and build your ownParameterizedrunner with this factory.IMHO this is a better design because the responsibility for creating the runner for a single data set is separated from the
Parameterizedrunner.It would be nice to have this pull request in JUnit 4.12. Otherwise the
createRunnerwill be part ofParameterized's public API.