Move test class validation aside

[ydewit]

I am extending ParentRunner<?> with my own implementation using DI and the single constructor check is getting in the way:

    public TestClass(Class klass) {
        fClass= klass;
        if (klass != null && klass.getConstructors().length > 1)
            throw new IllegalArgumentException(
                    "Test class can only have one constructor");

Could this be moved aside to a protected validateXXX method or even move it up to BlockJUnit4ClassRunner?

[dsaff]

Seems reasonable.

[dsaff]

@ydewit Sorry nothing ever happened on this. Did you eventually find a workaround?

[dhasday]

I was taking a quick look at this since it seemed pretty easy, but constructor validation was already moved to BlockJUnit4ClassRunner. Can this ticket be closed?

[marcphilipp]

ParentRunner still uses TestClass which still checks for a single constructor in its constructor. BlockJUnit4ClassRunner does check this, too. I don't see a reason to check this in multiple places.

@junit-team/junit-committers Any objections?

[dhasday]

Ah, missed that (and I guess the whole point of the issue).

My only concern with removing the check from TestClass is that usages of TestClass#getOnlyConstructor might not make sense since it wouldn't be guaranteed that the class only has 1 constructor.

[kcooney]

@dhasday We could have TestClass#getOnlyConstructor() do the check. I haven't looked at the callers to see if that would be a problem.

IIRC @bechte wanted to deprecate the TestClass constructor and add a static creational method that did momoization. We could do that, and have the static creational method not do the constructor check but have the public no-arg TestClass constructor as-is

[stefanbirkner]

Can somebody provide a use case for a test class that has more than one constructor. Which constructor would the runner select for creating an instance of the test class?

[bechte]

I wonder if we should support multiple constructors at all? If we do so we'd have to support all kind of side effects that come along with it:

• Constructor selection for test creation
• Resolve dependencies
• etc.

I wonder how we would figure this out? Normally we don't know nothing about the test classes' dependencies and, frankly, how would we find out?

I think we should stick with the test and requirement to perform any kind of setup in the @before annotated methods. Whereas I agree that this test should be extracted out to a single location.

According to what was said about the TestClass Factory: I really think we should turn the constructor of the TestClass deprecated and create a TestClass Factory. This would give us the freedom to keep TestClasses in a pool to save memory, perform requirement checks at a very single location and all this kind of stuff.

[adrian-tarau]

+1 to skip constructor check or allow subclasses (or a factory) to pick a constructor. My use case is the following:

• I'm using PowerMock + custom annotations
• I want to use TestCase to extract annotations and execute methods
• PowerMock instruments test classes and adds a second constructor (there is no way to disable this behaviour, which I think is wrong and I'm gonna create an issue with PowerMock team)
• TestCase fails because it requires a single constructor

Right now I'm cloning TestCase but ideally JUnit should make TestCase reusable.

[kcooney]

@adrian-tarau PowerMock is modifying your tests? I thought it only modified the classes under test?

Have you tried asking on StackOverflow to see how other Powermock users work around this problem?

[adrian-tarau]

There is no workaround, it's built in. You will not hit this "problem" unless you want to reuse TestCase provided with JUnit. If you scan yourself for annotations, that's not a problem.

[kcooney]

@adrian-tarau when you file the bug with the Powermock, please send us a link. I'm very curious.