[#SUREFIRE-44] Inner class inclusion too powerful - jira.codehaus.org

Maven Surefire

Inner class inclusion too powerful

Created: 05/Jun/06 08:37 AM Updated: 23/Nov/07 12:55 AM Resolved: 23/Nov/07 12:55 AM

Return to search

Component/s:

None

Affects Version/s:

2.0 (2.2 plugin)

Fix Version/s:

None

Time Tracking:

Not Specified

Issue Links:

Related

This issue relates to:

~~SUREFIRE-19~~ default excludes prevents inner class test cases

dependent

This issue is depended upon by:

~~SUREFIRE-288~~ Surefire tries to instantiate nested TestCase classes

Complexity:

Intermediate

Description

« Hide

When inner classes were included, it was done in a way that was too powerful. I believe it is too powerful in the following ways:

It is not name based – it assumes that all inner classes of an included class are test cases.
It does not work on nested classes (non-static classes)

See the test case and stack trace below.

I know people put tests in nested classes, but many nested classes exist for other reasons (simple mock objects, for example). Inner classes do not have no-arg constructors, and so including them includes classes that pretty much by definition will not be runnable.

As things stand, I cannot put a nested (non-static inner) class in a test case. I will get the exception below.

There needs to be some marker for nested test classes. If nothing else, nested classes without a no-arg constructor should be ignored.

But that still makes the inclusion very broad. The default inclusion of top-level classes is much more selective. I would prefer if the default inclusion of nested classes followed the same default naming pattern. But at the very least, please ignore classes without no-arg constructors.

I have been puzzling myself over errors like the following:

java.lang.NoSuchMethodException: com.hyphenhealth.edc.drq.editchecks.queries.compute.TestAThing$Foo.<init>()
at java.lang.Class.getConstructor0(Class.java:2647)
at java.lang.Class.getConstructor(Class.java:1629)
at org.apache.maven.surefire.battery.JUnitBattery.getTestConstructor(JUnitBattery.java:307)
at org.apache.maven.surefire.battery.JUnitBattery.processTestClass(JUnitBattery.java:150)
at org.apache.maven.surefire.battery.JUnitBattery.<init>(JUnitBattery.java:81)
at org.apache.maven.surefire.SurefireUtils.instantiateBattery(SurefireUtils.java:63)
at org.apache.maven.surefire.Surefire.instantiateBatteries(Surefire.java:262)
at org.apache.maven.surefire.Surefire.run(Surefire.java:140)
at org.apache.maven.surefire.Surefire.run(Surefire.java:87)
at org.apache.maven.surefire.Surefire.run(Surefire.java:63)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:585)
at org.apache.maven.surefire.SurefireBooter.main(SurefireBooter.java:785)
RUN ABORTED
java.lang.NoSuchMethodException

Test code:
public class TestAThing {
public class Foo { }
}

Description

When inner classes were included, it was done in a way that was too powerful. I believe it is too powerful in the following ways: It is not name based – it assumes that all inner classes of an included class are test cases. It does not work on nested classes (non-static classes) See the test case and stack trace below. I know people put tests in nested classes, but many nested classes exist for other reasons (simple mock objects, for example). Inner classes do not have no-arg constructors, and so including them includes classes that pretty much by definition will not be runnable. As things stand, I cannot put a nested (non-static inner) class in a test case. I will get the exception below. There needs to be some marker for nested test classes. If nothing else, nested classes without a no-arg constructor should be ignored. But that still makes the inclusion very broad. The default inclusion of top-level classes is much more selective. I would prefer if the default inclusion of nested classes followed the same default naming pattern. But at the very least, please ignore classes without no-arg constructors. I have been puzzling myself over errors like the following: java.lang.NoSuchMethodException: com.hyphenhealth.edc.drq.editchecks.queries.compute.TestAThing$Foo.<init>() at java.lang.Class.getConstructor0(Class.java:2647) at java.lang.Class.getConstructor(Class.java:1629) at org.apache.maven.surefire.battery.JUnitBattery.getTestConstructor(JUnitBattery.java:307) at org.apache.maven.surefire.battery.JUnitBattery.processTestClass(JUnitBattery.java:150) at org.apache.maven.surefire.battery.JUnitBattery.<init>(JUnitBattery.java:81) at org.apache.maven.surefire.SurefireUtils.instantiateBattery(SurefireUtils.java:63) at org.apache.maven.surefire.Surefire.instantiateBatteries(Surefire.java:262) at org.apache.maven.surefire.Surefire.run(Surefire.java:140) at org.apache.maven.surefire.Surefire.run(Surefire.java:87) at org.apache.maven.surefire.Surefire.run(Surefire.java:63) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.apache.maven.surefire.SurefireBooter.main(SurefireBooter.java:785) RUN ABORTED java.lang.NoSuchMethodException Test code: public class TestAThing { public class Foo { } }

Show »

Sort Order:

Ken Arnold made changes - 05/Jun/06 08:38 AM

Field	Original Value	New Value
Link		This issue is related to ~~SUREFIRE-19~~ [ ~~SUREFIRE-19~~ ]

[ Permalink | « Hide ]

Ken Arnold added a comment - 05/Jun/06 08:38 AM

I suspect that adding this feature is what created the bug.

[ Permalink | « Hide ]

Ken Arnold added a comment - 05/Jun/06 08:53 AM

I thought I had a workaround – make classes static, etc. But I don't. Because of things like this:

public interface MockExpressionMatcher {
MockExpressionMatcher MATCH_ALL = new MockExpressionMatcher() {
public boolean matches(String sql) { return true; }
};

boolean matches(String sql);
}

When I use it in code, the anonymous object for MATCH_ALL is created in that class. Or at least it seems so. Consider:

private MockDBConnection setupTestDB() { MockDBConnection c = new MockDBConnection(); c.addResultSetFactory(MockExpressionMatcher.MATCH_ALL, new MockUpdateFactory()); return c; }

I then get the following problem when running tests:

java.lang.NoSuchMethodException: com.hyphenhealth.edc.drq.editchecks.queries.TestComputeQuery$1.<init>()

This is true – a javap of this anonymous class shows no constructor at all:

class com.hyphenhealth.edc.drq.editchecks.queries.TestComputeQuery$1 extends java.lang.Object{
}

I cannot control this compiler behavior. This is how it handles static final constructed objects (in interfaces anyway). I cannot make this class non-static because this is not my file. (I probably could make a nested static class for each such field if I controlled the file).

If there is a language for exclusion of nested classes I cannot find it in the documentation. So I'm out of workarounds for now.

(This is partly a consequence of naming java files instead of classes for inclusion/exclusion. If classes were named I could exclude */\$* (all nested classes). But nested classes have no separate file to include/exclude. I will file a separate feature request for that.)

[ Permalink | « Hide ]

Ken Arnold added a comment - 08/Jun/06 11:09 AM

My mistake, this is not a 2.0 issue (that was a typo). It's a 1.5.3 issue.

I am going to have to add no-arg constructors to all nested classes right now. This is just wrong.

Brett Porter made changes - 31/Jul/06 09:09 AM

Fix Version/s

2.1 [ 12620 ]

[ Permalink | « Hide ]

Carlos Sanchez added a comment - 14/Dec/06 05:11 AM

do you mean you are having this issue in 1.5.3 and it's already fixed in 2.0 ?

[ Permalink | « Hide ]

Ken Arnold added a comment - 17/Dec/06 11:45 PM

No, I meant that I was using 1.5.3 when I had the problem, and that I hadn't tried it at 2.0. Sorry for the unclarity.

Stephen Coy made changes - 20/Feb/07 07:55 AM

Link

This issue is depended upon by ~~SUREFIRE-288~~ [ ~~SUREFIRE-288~~ ]

Brett Porter made changes - 23/Feb/07 01:05 AM

Fix Version/s		2.4 [ 13243 ]
Fix Version/s	2.3 [ 12620 ]

[ Permalink | « Hide ]

Dan Fabulich added a comment - 23/Nov/07 12:55 AM

Can't reproduce; integration checked in revision 597570

Dan Fabulich made changes - 23/Nov/07 12:55 AM

Resolution		Cannot Reproduce [ 5 ]
Fix Version/s	2.4 [ 13243 ]
Status	Open [ 1 ]	Closed [ 6 ]