I want to ask if people are comfortable with merging CASSANDRA-17964 before 4.1 is out. We clean a lot of things and rename tests which we should take care of. (like PaxosRepairTest2).
We might still fix it without merging 17964 as we know what should be fixed and we can wait until 4.1 is out but I personally do not see any reason for this. Thoughts? ________________________________________ From: Miklosovic, Stefan <stefan.mikloso...@netapp.com> Sent: Monday, November 7, 2022 23:23 To: dev@cassandra.apache.org Subject: Re: Some tests are never executed in CI due to their name Hi, A while ago we came up with a very nice solution (thank you David Capwell for suggesting this). Sorry for no updating this thread sooner, a lot of discussion was done in the ticket. (CASSANDRA-17964). The solution we think is the best is to have an Ant task which would scan all classes via reflection on build-test target (separate / dedicated target will be available too) and it will scan files on containing Test annotation. If that annotation is present, it means that class is a test and it has to have so and so naming convention (currently it has to end on Test). There are details covered as well (for example, cases when there is an abstract class containing all @Test methods and a concrete test class is just extending that, all is covered too). The reason for this approach is that: 1) it is way more robust that trying to come up with some super regexp 2) We can put arbitrary logic into it, for example, we may ignore the invalid name of a test class if we "know what we are doing" but putting @IgnoreInvalidTestName annotation on top of that class and Ant task would just skip that. 3) We can introduce this task into all branches we support. Checkstyle is present only in 4.1 and trunk so we would have kind of half-baked solution here and older branches would not be covered. As we enabled more tests because they were ignored / effectively skipped previously, some enabled tests started to fail. There are two (currently know): (1) This one started to fail on 4.0, not sure whats the deal for older branches, it probably needs deeper look (2) PaxorRepairTest2 - this one fails to start because of configuration in that test but even it is fixed it fails anyway on something else. I will create a ticket for that soon. I will reference this email to Cassandra update status to raise awareness that some tests we enabled started to fail in 4.0 -> trunk. 1) https://issues.apache.org/jira/browse/CASSANDRA-18021 2) ticket to be created Thanks ________________________________________ From: Berenguer Blasi <berenguerbl...@gmail.com> Sent: Thursday, November 3, 2022 9:47 To: dev@cassandra.apache.org Subject: Re: Some tests are never executed in CI due to their name NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. Re '@Test' without 'abstract': There's a corner case of a base abstract class with all test cases. Then 2 or 3 inheriting classes with no '@Test' annotations bc they just run the base class with different configs, setups, envs, parametrized runs, etc I've seen some of those. On 1/11/22 22:54, Mick Semb Wever wrote: Aha wait a minute I got it :D Yeah. This is not what you invented, it is already in, sorry. So, you are saying we should just modify this to cover @Test / abstract. I dont know ... interesting. Lets wait for others to answer. This thread is rather painful to digest because of the top-posting :'( I agree that all non-abstract classes that contain a @test annotation must have a filename ending in "Test.java" That leaves CI scripts untouched. How we enforce that I have no opinion about. If it's not easy in checkstyle, just do it with some bash in the cassandra-test.sh script. It'll get caught pre-commit either way, and once the rename of existing files is done the precedence is clearly in place.
