Hi list, it was reported that some tests are not executed as part of CI because of their name (1). There is (2) and (3) so right now it runs only "*Test.java" files.
Except of us renaming the affected classes, we should fix this in a more robust way - via checkstyle - so we enforce how test files have to be called. We came up with two approaches and we do not know which one to choose so we want to gather more feedback / opinions. The first approach would make this happen (implemented here (4)) class name - passes / fails the checkstyle TestIt - fails TestsThatFeature - fails ThisTestsMyFeature - fails SomeTests - fails SomeHelperClassForTesting - passes SomeTest - passes DistributedTestBase - passes MyTestSplit1 - passes We would fix "SplitN" test files by hand (I think they are not executed right now in CI either) to something like MySplit1Test and MySplit2Test but the regexp we would eventually use would not prevent this from happening in the future. I do not know how to construct such a complex regexp yet which would cover all cases above plus this splitting one. Feel free to look into the ticket where details are discussed. The second approach would forbid to have any occurrence of "test" in the file name but at the end. For example, it would fail on classes like these MyTestSplit1 TestHelper BurnTestUtil FuzzTestBase DistributedTestSnitch CASCommonTestCases ServerTestUtils AuthTestUtils TestMemtable While this would also fix "MyTestSplitN" cases, I consider this to be way more invasive to the current codebase because we would have to rename a lot of files (like, hundreds, plus a lot of places they are referenced / used at) and I do not even think this is necessary and desirable. Like, how would we call "FuzzTestBase" to not contain "Test" in it? I think having "test" string in the middle is just fine, specifically for helper classes. One possible fix would be to replace "Test" with "Tester" so we would have "TesterMemtable", "AuthTesterUtils", "TesterHelper", "DistributedTesterSnitch" ... Word "Tester" would be explicitly allowed. I personally lean to the first approach even though cases like "MyTestSplit1" will not be spotted in compile time, but hey, we still do have a review process on top of this. The obvious violations like "TestThisFeature" and "MySuperTests" would be evaluated as violations which I would argue happens the most often. Please keep in mind that we have checkstyle only in 4.1 and trunk. So, while we would fix names of classes in all branches we support so they are picked up by CI, checkstyle for this would be introduced in 4.1 and trunk only. (1) https://issues.apache.org/jira/browse/CASSANDRA-17964 (2) https://github.com/apache/cassandra/blob/trunk/build.xml#L61 (3) https://github.com/apache/cassandra/blob/trunk/build.xml#L1033 (4) https://github.com/instaclustr/cassandra/commits/CASSANDRA-17964-4.1-2
