Issue Details (XML | Word | Printable)

Key: GRAILS-2469
Type: Test Test
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Graeme Rocher
Reporter: Jason Rudolph
Votes: 1
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Grails

Patch: Build Fails to Run All Tests

Created: 16/Feb/08 02:53 PM   Updated: 13/Mar/08 10:37 AM   Resolved: 13/Mar/08 10:37 AM
Return to search
Component/s: ContinuousBuild, Testing
Affects Version/s: 1.0, 1.0.1
Fix Version/s: 1.0.2

Time Tracking:
Not Specified

File Attachments: 1. File include_missing_tests_in_build_v2.diff (75 kB)

Environment: Mac OS X 10.5.1, JDK 5

Patch Submitted: Yes


 Description  « Hide

The current Grails build script only runs tests where the class name ends in "Tests". See ant/build/unit-test.xml.

<include name="**/${test}Tests.class" />

The build excludes the several tests whose class name ends in "Test". The attached patch corrects that issue, first by updating ant/build/unit-test.xml.

<include name="**/${test}Test.class" />
<include name="**/${test}Tests.class" />

Also, Grails currently includes some classes that are not technically tests, but they still end in "Test". For example, test/persistence/org/codehaus/groovy/grails/domain/ManyToManyTest.groovy is a sample domain class used to support other test cases. This class has no test cases of its own. This patch renames that class to SampleManyToMany.groovy. Doing so prevents JUnit from looking for tests in that class (since it's not even intended to include test cases). Without that change, JUnit will expect the class to include at least one test, and it will report an error otherwise.

Grails currently includes a handful of these "sample" classes that needed to be renamed. This patch renames those classes accordingly.

The patch improves the Grails build by:

  1. Ensuring that all test cases are run with every build!
  2. Improving the code coverage (since the missing tests are now part of the build).

Regarding code coverage, here's a quick example of how these missing tests were resulting in inaccurate code coverage.

Look at #getFormatFromURI in the latest Grails build. Observe that it lacks proper coverage. However, if you look at #testGetFormatFromURI in WebUtilsTest.groovy, you can see that it provides the missing coverage. Because the current Grails build excludes this test, the coverage report wrongly reports this line as lacking coverage. This patch corrects that problem and many similar problems.



Jason Rudolph added a comment - 16/Feb/08 02:55 PM

Corrected link to current coverage report for WebUtils.java.


Peter Ledbrook added a comment - 17/Feb/08 12:20 PM

Should we not rename the tests so that they match the convention? Also, I would consider removing ManyToManyTest since the useful code appears to be commented out. In general though, I think the rename is a good idea. BTW, SampleUnitOneToMany.groovy should be SampleUniOneToMany.groovy.


Jason Rudolph added a comment - 17/Feb/08 12:59 PM

Hi Peter,
I agree that the tests should follow a consistent naming convention. I'd consider that to be a superior enhancement. If the team agrees, I'm happy to submit a patch that makes that change instead of updating ant/build/unit-test.xml.

Cheers,
Jason


Graeme Rocher added a comment - 18/Feb/08 04:16 AM

Agreed this would be a better patch


Graeme Rocher added a comment - 18/Feb/08 04:17 AM

In the meantime i've renamed WebUtilsTest (ie added the s)


Jason Rudolph added a comment - 18/Feb/08 07:51 AM

Thanks, Graeme. I'll put together an updated patch and post it here.


Jason Rudolph added a comment - 18/Feb/08 07:55 PM

The updated patch renames the remaining test classes ending in "Test" to instead end in "Tests", thus making all Grails test classes follow a consistent naming convention.

test/persistence/org/codehaus/groovy/grails/orm/hibernate/cfg/GrailsDomainConfigurationUtilTest.java
test/commons/org/codehaus/groovy/grails/validation/routines/RegexValidatorTest.java
test/commons/org/codehaus/groovy/grails/validation/routines/UrlValidatorTest.java
test/commons/org/codehaus/groovy/grails/validation/routines/InetAddressValidatorTest.java
test/commons/org/codehaus/groovy/grails/validation/routines/DomainValidatorTest.java

The patch no longer modifies the build script (as the renaming process above removes the need to change the build script).


Graeme Rocher added a comment - 13/Mar/08 10:37 AM

Thanks for the patch!