Change the naming convention for AllITs

Development
#1

Hi devs,

I’d like to propose to change our test naming convention for the All IT test which runs all the IT tests of a module. Right now we use AllITs. I’d like to propose that we move to AllIT.

The main reason is that there’s a SonarQube rule that uses ^((Test|IT)[a-zA-Z0-9_]+|[A-Z][a-zA-Z0-9_]*(Test|Tests|TestCase|IT|ITCase))$ as the default and I’d rather we comply than have to customize the rule.

I see no strong reason to add the ending s.

WDYT?

Thanks

PS: If agreed, I’ll take care of renaming the tests and updating xwiki-commons/pom.xml at a93f8d5aa2f8939c77f6b0705c3e222a20579c2f · xwiki/xwiki-commons · GitHub

#2

Forgot to mention that right now SonarQube reports violations for all our AllITs tests because of this.

#3

Globally +1

There’s a possible impact on contrib extension using AllITs when they’ll upgrade the parent pom, right?

#4

+1 as well, with the same concern regarding the impact for contrib extensions

#5

Yes, it means that when contrib extensions upgrade their parent pom to XWiki 15.4, they’ll also need to rename AllITs to AllIT. I don’t see that as a big problem and we have very few contrib extensions having integration tests.

Thanks

#6

+1

#7

Extension authors will need to be careful though since if they don’t rename AllITs, the failsafe plugin will not execute AllITs anymore after the parent upgrade and they might not realize it.

#8

What we really need is a build check that fails the build if there are some test classes (ie classes with @Test annotation that are not executed by surefire or failsafe).

#9

ok, one simple way to fix just the contrib extension issue would be to use Maven Failsafe Plugin – failsafe:verify , i.e. update the contrib extensions having IT tests with it.

#10

It’s not very easy to implement since it would need to support Nested tests in JUnit5.

For ex, failsafe generates:

TEST-org.xwiki.whatsnew.test.ui.AllITs.xml:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report-3.0.xsd" version="3.0" name="org.xwiki.whatsnew.test.ui.AllITs" time="61.602" tests="0" errors="0" skipped="0" failures="0">
  <properties>...  </properties>
</testsuite>

And TEST-org.xwiki.whatsnew.test.ui.AllITs$NestedWhatsNewIT.xml:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-failsafe-plugin/xsd/failsafe-test-report-3.0.xsd" version="3.0" name="org.xwiki.whatsnew.test.ui.AllITs$NestedWhatsNewIT" time="5.304" tests="1" errors="0" skipped="0" failures="0">
  <properties>...  </properties>
  <testcase name="verify(TestUtils)" classname="org.xwiki.whatsnew.test.ui.AllITs$NestedWhatsNewIT" time="5.298">
    <system-out>...</system-out>
    <system-err>...</system-err>
  </testcase>
</testsuite>

And it’s not that easy to reconcile org.xwiki.whatsnew.test.ui.WhatsNewIT with org.xwiki.whatsnew.test.ui.AllITs$NestedWhatsNewIT. For that the check will need to parse AllIT and know the concept of nested tests (i.e parse @Nested) to extract out NestedWhatsNewIT from:

    @Nested
    @DisplayName("What's New UI")
    class NestedWhatsNewIT extends WhatsNewIT
    {
    }
#11

Actually, a better way is maybe to have a globally registered JUnit5 extension that records executed tests and outputs its own format (understanding nested tests) so that it can more easily be compared to test classes in a check later on.

#12

Done:

#13

Also merged on 14.10.x branch.

#14

Also updated all “flickering test” custom fields in JIRA that were using “AllITs” and that were still open.