Maven Surefire
  1. Maven Surefire
  2. SUREFIRE-587

Doesn't run tests if they don't end with Test

    Details

    • Type: Wish Wish
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.4.3
    • Fix Version/s: None
    • Labels:
      None
    • Environment:
      N/A
    • Complexity:
      Intermediate
    • Number of attachments :
      0

      Description

      We have a mix of test - some purely junit4 (e.g. not extending anything and annotating with @Test) - and some which extends TestCase.
      I've discovered that the ones extending TestCase won't run if the classname doesn't end with Test - so that the class CommonsAttributesParserTests won't get included (notice the ending "s").

        Activity

        Hide
        Greg Symons added a comment -

        I've hit this problem on pure JUnit4 tests as well.

        Show
        Greg Symons added a comment - I've hit this problem on pure JUnit4 tests as well.
        Hide
        Gareth Clay added a comment -

        I've hit this issue with only pure JUnit4 tests too. Our build machine is currently only running 27 of our 120+ tests (those with class names that begin or end in 'Test').

        As I understand it, the only requirement for a JUnit4 test to be run is the presence of the @Test annotation on the test method. The name of the class and the (non)presence of @RunWith on the class should make no difference. The JUnit4 runner in Eclipse follows this behaviour, and correctly identifies all of our tests.

        All this with JUnit4.4, and surefire 2.7-SNAPSHOT as at 20101119 (since I'd been hoping this had been fixed as part of SUREFIRE-482, but it doesn't seem that way).

        Show
        Gareth Clay added a comment - I've hit this issue with only pure JUnit4 tests too. Our build machine is currently only running 27 of our 120+ tests (those with class names that begin or end in 'Test'). As I understand it, the only requirement for a JUnit4 test to be run is the presence of the @Test annotation on the test method. The name of the class and the (non)presence of @RunWith on the class should make no difference. The JUnit4 runner in Eclipse follows this behaviour, and correctly identifies all of our tests. All this with JUnit4.4, and surefire 2.7-SNAPSHOT as at 20101119 (since I'd been hoping this had been fixed as part of SUREFIRE-482 , but it doesn't seem that way).
        Hide
        Gareth Clay added a comment -

        Hmm... Having taken a very brief look at the code, it seems that surefire's JUnit4TestChecker.isValidJUnit4Test() method is doing the right thing. So it's just not getting the right list of test classes to apply that filtering to.

        Following on from that, I've successfully worked around this issue by explicitly adding

        <includes>
        <include>*/.class</include>
        </includes>

        to my POM. I think this should be the default implicit behaviour - it makes no sense to filter by class name when the only requirement for inclusion is an @Test annotation.

        Show
        Gareth Clay added a comment - Hmm... Having taken a very brief look at the code, it seems that surefire's JUnit4TestChecker.isValidJUnit4Test() method is doing the right thing. So it's just not getting the right list of test classes to apply that filtering to. Following on from that, I've successfully worked around this issue by explicitly adding <includes> <include>* / .class</include> </includes> to my POM. I think this should be the default implicit behaviour - it makes no sense to filter by class name when the only requirement for inclusion is an @Test annotation.
        Hide
        Kristian Rosenvold added a comment -

        I will be treating this as a documentation issue and update the FAQ wrt this issue, since it's already specified in the main documentation at http://maven.apache.org/plugins/maven-surefire-plugin/examples/inclusion-exclusion.html
        . You have to update the include/exclude pattern to include all your classes if you want surefire to scan all your project classes for annotations. I will close this issue when the faq is updated.

        Show
        Kristian Rosenvold added a comment - I will be treating this as a documentation issue and update the FAQ wrt this issue, since it's already specified in the main documentation at http://maven.apache.org/plugins/maven-surefire-plugin/examples/inclusion-exclusion.html . You have to update the include/exclude pattern to include all your classes if you want surefire to scan all your project classes for annotations. I will close this issue when the faq is updated.
        Hide
        Gareth Clay added a comment -

        Apologies, so it is. I missed that.

        I would still argue that scanning all classes is a more useful default. JUnit imposes no restriction on test class naming, so it seems odd that surefire filters by name by default. Wouldn't a better default be to scan all classes, and let users impose their own restrictions if they're using a particular naming convention? It would avoid confusion and issues like this being raised, and I would have thought the performance hit is likely to be small on all but very large codebases.

        Show
        Gareth Clay added a comment - Apologies, so it is. I missed that. I would still argue that scanning all classes is a more useful default. JUnit imposes no restriction on test class naming, so it seems odd that surefire filters by name by default. Wouldn't a better default be to scan all classes, and let users impose their own restrictions if they're using a particular naming convention? It would avoid confusion and issues like this being raised, and I would have thought the performance hit is likely to be small on all but very large codebases.
        Hide
        Matthew Gilliard added a comment -

        Agree with Gareth's comment – this has caught us out in the past, too. I could provide a patch, unless there is a backwards-compatibility concern?

        Show
        Matthew Gilliard added a comment - Agree with Gareth's comment – this has caught us out in the past, too. I could provide a patch, unless there is a backwards-compatibility concern?
        Hide
        Kristian Rosenvold added a comment -

        I have changed my mind too; and I think we'll try to fix this. But I think we'll probably take it together with any other changes to default values for "some future" release so we make one release that does all the modifications.

        Show
        Kristian Rosenvold added a comment - I have changed my mind too; and I think we'll try to fix this. But I think we'll probably take it together with any other changes to default values for "some future" release so we make one release that does all the modifications.
        Hide
        Christian Gnuechtel added a comment -

        Nearly one year has been passed after the last comment. Are there any plans to implement this feature?

        If not, the workaround above is ok. But it has to be:

        <include>**/*.class</include>
        Show
        Christian Gnuechtel added a comment - Nearly one year has been passed after the last comment. Are there any plans to implement this feature? If not, the workaround above is ok. But it has to be: <include>**/*.class</include>

          People

          • Assignee:
            Unassigned
            Reporter:
            David J. M. Karlsen
          • Votes:
            4 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated: