Some tests are never executed in CI due to their name

[Miklosovic, Stefan]

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

Re: Some tests are never executed in CI due to their name

[Derek Chen-Becker]

For clarity, what is the final regex you've landed on for the first
approach? Is it "^(?!(?:Test))(?!.*?(?:Tests)).*$" ? Are we just trying to
reject anything that *starts* with "Test" or contains "Tests" somewhere in
the name? It might be more clear to state what we think a valid test name
should be, not in regex form.

When it comes to "TestBase" files, are there any of these that are actually
intended to be run as a test? The "Base" would indicate to me that it's
intended to be inherited, not run directly. Do we have some "TestBase"
classes somewhere that aren't inherited and are meant to be run directly,
because that definitely seems like a misleading name that should be fixed,
invasive or not. I would lean toward the principle of least surprise here,
even if it means an involved one-time cleanup.

Cheers,

Derek

On Mon, Oct 24, 2022 at 3:02 AM Miklosovic, Stefan <
stefan.mikloso...@netapp.com> wrote:

> 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



-- 
+---+
| Derek Chen-Becker |
| GPG Key available at https://keybase.io/dchenbecker and   |
| https://pgp.mit.edu/pks/lookup?search=derek%40chen-becker.org |
| Fngrprnt: EB8A 6480 F0A3 C8EB C1E7  7F42 AFC5 AFEE 96E4 6ACC  |
+---+

Re: Some tests are never executed in CI due to their name

[Miklosovic, Stefan]

Hi Derek,

yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$

What we are trying to achieve by that is to have a test which ends on "Test" 
and nothing else and it may contain "Test" only in case it does not start with 
it.

To your second paragraph, I do not think we have any *Base files which are 
tests (that would be a bug) nor they are standalone (not inherited). By first 
approach, TestBaseImpl (which we do have) would start to be invalid, we would 
have to rename it to "DistributedTestBaseImpl" because it was starting on 
"Test" which is no-no. I agree that "TestBaseImpl" is rather unfortunate name. 
Currently, TestBaseImpl itself extends "DistributedTestBase" so in the end we 
would have "DistributedTestBaseImpl -> TestBaseImpl -> DistributedTestBase". 
Please keep in mind that "DistributedTestBase" is located in dtest jar 
(separate project) and that is rather inconvenient to rename.

Regards


From: Derek Chen-Becker 
Sent: Monday, October 24, 2022 15:09
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.



For clarity, what is the final regex you've landed on for the first approach? 
Is it "^(?!(?:Test))(?!.*?(?:Tests)).*$" ? Are we just trying to reject 
anything that *starts* with "Test" or contains "Tests" somewhere in the name? 
It might be more clear to state what we think a valid test name should be, not 
in regex form.

When it comes to "TestBase" files, are there any of these that are actually 
intended to be run as a test? The "Base" would indicate to me that it's 
intended to be inherited, not run directly. Do we have some "TestBase" classes 
somewhere that aren't inherited and are meant to be run directly, because that 
definitely seems like a misleading name that should be fixed, invasive or not. 
I would lean toward the principle of least surprise here, even if it means an 
involved one-time cleanup.

Cheers,

Derek

On Mon, Oct 24, 2022 at 3:02 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>> wrote:
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://g

Re: Some tests are never executed in CI due to their name

[Miklosovic, Stefan]

I meant MyFeatureTest -> DistributedTestBaseImpl  -> DistributedTestBase

TestBaseImpl would be renamed to DistributedTestBaseImpl

DistributedTestBase is in dtest package (separate project / dependency)


From: Miklosovic, Stefan 
Sent: Monday, October 24, 2022 15:26
To: dev@cassandra.apache.org
Subject: Re: Some tests are never executed in CI due to their name

Hi Derek,

yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$

What we are trying to achieve by that is to have a test which ends on "Test" 
and nothing else and it may contain "Test" only in case it does not start with 
it.

To your second paragraph, I do not think we have any *Base files which are 
tests (that would be a bug) nor they are standalone (not inherited). By first 
approach, TestBaseImpl (which we do have) would start to be invalid, we would 
have to rename it to "DistributedTestBaseImpl" because it was starting on 
"Test" which is no-no. I agree that "TestBaseImpl" is rather unfortunate name. 
Currently, TestBaseImpl itself extends "DistributedTestBase" so in the end we 
would have "DistributedTestBaseImpl -> TestBaseImpl -> DistributedTestBase". 
Please keep in mind that "DistributedTestBase" is located in dtest jar 
(separate project) and that is rather inconvenient to rename.

Regards


From: Derek Chen-Becker 
Sent: Monday, October 24, 2022 15:09
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.



For clarity, what is the final regex you've landed on for the first approach? 
Is it "^(?!(?:Test))(?!.*?(?:Tests)).*$" ? Are we just trying to reject 
anything that *starts* with "Test" or contains "Tests" somewhere in the name? 
It might be more clear to state what we think a valid test name should be, not 
in regex form.

When it comes to "TestBase" files, are there any of these that are actually 
intended to be run as a test? The "Base" would indicate to me that it's 
intended to be inherited, not run directly. Do we have some "TestBase" classes 
somewhere that aren't inherited and are meant to be run directly, because that 
definitely seems like a misleading name that should be fixed, invasive or not. 
I would lean toward the principle of least surprise here, even if it means an 
involved one-time cleanup.

Cheers,

Derek

On Mon, Oct 24, 2022 at 3:02 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>> wrote:
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

Re: Some tests are never executed in CI due to their name

[Derek Chen-Becker]

OK, that makes sense, and I missed the rename of "TestBaseImpl" (which I
agree is in need of fixing). When you say:

> What we are trying to achieve by that is to have a test which ends on
"Test" and nothing else and it may contain "Test" only in case it does not
start with it.

Should the regex enforce the "which ends on Test" part? Otherwise, if we
want to allow tests like MyTestSplit1, do we need to update the "test.name"
parameter to allow that in the test runner?

Thanks,

Derek


On Mon, Oct 24, 2022 at 7:27 AM Miklosovic, Stefan <
stefan.mikloso...@netapp.com> wrote:

> Hi Derek,
>
> yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$
>
> What we are trying to achieve by that is to have a test which ends on
> "Test" and nothing else and it may contain "Test" only in case it does not
> start with it.
>
> To your second paragraph, I do not think we have any *Base files which are
> tests (that would be a bug) nor they are standalone (not inherited). By
> first approach, TestBaseImpl (which we do have) would start to be invalid,
> we would have to rename it to "DistributedTestBaseImpl" because it was
> starting on "Test" which is no-no. I agree that "TestBaseImpl" is rather
> unfortunate name. Currently, TestBaseImpl itself extends
> "DistributedTestBase" so in the end we would have "DistributedTestBaseImpl
> -> TestBaseImpl -> DistributedTestBase". Please keep in mind that
> "DistributedTestBase" is located in dtest jar (separate project) and that
> is rather inconvenient to rename.
>
> Regards
>
> 
> From: Derek Chen-Becker 
> Sent: Monday, October 24, 2022 15:09
> 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.
>
>
>
> For clarity, what is the final regex you've landed on for the first
> approach? Is it "^(?!(?:Test))(?!.*?(?:Tests)).*$" ? Are we just trying to
> reject anything that *starts* with "Test" or contains "Tests" somewhere in
> the name? It might be more clear to state what we think a valid test name
> should be, not in regex form.
>
> When it comes to "TestBase" files, are there any of these that are
> actually intended to be run as a test? The "Base" would indicate to me that
> it's intended to be inherited, not run directly. Do we have some "TestBase"
> classes somewhere that aren't inherited and are meant to be run directly,
> because that definitely seems like a misleading name that should be fixed,
> invasive or not. I would lean toward the principle of least surprise here,
> even if it means an involved one-time cleanup.
>
> Cheers,
>
> Derek
>
> On Mon, Oct 24, 2022 at 3:02 AM Miklosovic, Stefan <
> stefan.mikloso...@netapp.com> wrote:
> 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", "TesterHe

Re: Some tests are never executed in CI due to their name

[Miklosovic, Stefan]

We do not want to allow MyTestSplit1. That would be forbidden but current 
regexp would not catch it. There is a gap here I was talking about. This is the 
case of "test" in the middle which should be possible (DistributedTestBaseImpl) 
but then we need to fix cases like these and detect this manually on a review.

Feel free to come up with better regexp if you want but I think that catching 
MyTestSplit1 as illegal would require massive rename of all other classes which 
happen to contain "test" in their names.


From: Derek Chen-Becker 
Sent: Monday, October 24, 2022 15:53
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.



OK, that makes sense, and I missed the rename of "TestBaseImpl" (which I agree 
is in need of fixing). When you say:

> What we are trying to achieve by that is to have a test which ends on "Test" 
> and nothing else and it may contain "Test" only in case it does not start 
> with it.

Should the regex enforce the "which ends on Test" part? Otherwise, if we want 
to allow tests like MyTestSplit1, do we need to update the 
"test.name" parameter to allow that in the test runner?

Thanks,

Derek


On Mon, Oct 24, 2022 at 7:27 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>> wrote:
Hi Derek,

yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$

What we are trying to achieve by that is to have a test which ends on "Test" 
and nothing else and it may contain "Test" only in case it does not start with 
it.

To your second paragraph, I do not think we have any *Base files which are 
tests (that would be a bug) nor they are standalone (not inherited). By first 
approach, TestBaseImpl (which we do have) would start to be invalid, we would 
have to rename it to "DistributedTestBaseImpl" because it was starting on 
"Test" which is no-no. I agree that "TestBaseImpl" is rather unfortunate name. 
Currently, TestBaseImpl itself extends "DistributedTestBase" so in the end we 
would have "DistributedTestBaseImpl -> TestBaseImpl -> DistributedTestBase". 
Please keep in mind that "DistributedTestBase" is located in dtest jar 
(separate project) and that is rather inconvenient to rename.

Regards


From: Derek Chen-Becker mailto:de...@chen-becker.org>>
Sent: Monday, October 24, 2022 15:09
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.



For clarity, what is the final regex you've landed on for the first approach? 
Is it "^(?!(?:Test))(?!.*?(?:Tests)).*$" ? Are we just trying to reject 
anything that *starts* with "Test" or contains "Tests" somewhere in the name? 
It might be more clear to state what we think a valid test name should be, not 
in regex form.

When it comes to "TestBase" files, are there any of these that are actually 
intended to be run as a test? The "Base" would indicate to me that it's 
intended to be inherited, not run directly. Do we have some "TestBase" classes 
somewhere that aren't inherited and are meant to be run directly, because that 
definitely seems like a misleading name that should be fixed, invasive or not. 
I would lean toward the principle of least surprise here, even if it means an 
involved one-time cleanup.

Cheers,

Derek

On Mon, Oct 24, 2022 at 3:02 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>>>
 wrote:
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 discuss

Re: Some tests are never executed in CI due to their name

[Miklosovic, Stefan]

One more point as I was reading your email in a hurry. The ultimate goal is to 
run all tests. So even if we apply the proposed regexp in checkstyle and there 
is an unfortunate case that MyTestSplit1 would slip through on a review and it 
would not be executed in CI, if we do not figure out some better regexp, we 
might relax "test.name" property to include Split*.java as well. Yes, this 
might be also done.

Enforcing "all tests to end on "Test" " means that we would need to know which 
.java files contain tests and which not without looking into them. I do not 
think that is possible under circumstances we have where (checkstyle module).


From: Derek Chen-Becker 
Sent: Monday, October 24, 2022 15:53
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.



OK, that makes sense, and I missed the rename of "TestBaseImpl" (which I agree 
is in need of fixing). When you say:

> What we are trying to achieve by that is to have a test which ends on "Test" 
> and nothing else and it may contain "Test" only in case it does not start 
> with it.

Should the regex enforce the "which ends on Test" part? Otherwise, if we want 
to allow tests like MyTestSplit1, do we need to update the 
"test.name" parameter to allow that in the test runner?

Thanks,

Derek


On Mon, Oct 24, 2022 at 7:27 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>> wrote:
Hi Derek,

yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$

What we are trying to achieve by that is to have a test which ends on "Test" 
and nothing else and it may contain "Test" only in case it does not start with 
it.

To your second paragraph, I do not think we have any *Base files which are 
tests (that would be a bug) nor they are standalone (not inherited). By first 
approach, TestBaseImpl (which we do have) would start to be invalid, we would 
have to rename it to "DistributedTestBaseImpl" because it was starting on 
"Test" which is no-no. I agree that "TestBaseImpl" is rather unfortunate name. 
Currently, TestBaseImpl itself extends "DistributedTestBase" so in the end we 
would have "DistributedTestBaseImpl -> TestBaseImpl -> DistributedTestBase". 
Please keep in mind that "DistributedTestBase" is located in dtest jar 
(separate project) and that is rather inconvenient to rename.

Regards


From: Derek Chen-Becker mailto:de...@chen-becker.org>>
Sent: Monday, October 24, 2022 15:09
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.



For clarity, what is the final regex you've landed on for the first approach? 
Is it "^(?!(?:Test))(?!.*?(?:Tests)).*$" ? Are we just trying to reject 
anything that *starts* with "Test" or contains "Tests" somewhere in the name? 
It might be more clear to state what we think a valid test name should be, not 
in regex form.

When it comes to "TestBase" files, are there any of these that are actually 
intended to be run as a test? The "Base" would indicate to me that it's 
intended to be inherited, not run directly. Do we have some "TestBase" classes 
somewhere that aren't inherited and are meant to be run directly, because that 
definitely seems like a misleading name that should be fixed, invasive or not. 
I would lean toward the principle of least surprise here, even if it means an 
involved one-time cleanup.

Cheers,

Derek

On Mon, Oct 24, 2022 at 3:02 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>>>
 wrote:
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 

Re: Some tests are never executed in CI due to their name

[Derek Chen-Becker]

OK, I think the approach you're proposing is fine. As an orthogonal idea,
is there any way to do a one-time audit to narrow down files that we need
to inspect, or any way to automate confirmation of a file containing tests?
For example:

* Start with all of the files under the `test` directory (is it safe to
assume that this is the only location for test classes?) => 1457 files
* Remove any files that already end in "Test.java" => down to 396 files
* Is it safe to assume that only files having "Test" somewhere in their
name are test classes? That reduces the list down to 60 files

Alternatively, are we only talking about unit tests? Would it be sufficient
to look for files that don't end in "Test.java" but contain "@Test"
annotations? That's a significantly smaller set, so maybe we could fix unit
tests first:

❯❯❯ for fl in test/unit/**/*.java; do if [[ $fl != *Test.java ]]; then if
grep -qF '@Test' $fl; then print $fl; fi; fi; done

 circleci-parity ✱ ◼
test/unit/org/apache/cassandra/auth/CassandraAuthorizerTruncatingTests.java
test/unit/org/apache/cassandra/cql3/BatchTests.java
test/unit/org/apache/cassandra/cql3/KeywordTestBase.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexTester.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
test/unit/org/apache/cassandra/dht/PartitionerTestCase.java
test/unit/org/apache/cassandra/utils/UUIDTests.java

Cheers,

Derek


On Mon, Oct 24, 2022 at 9:00 AM Miklosovic, Stefan <
stefan.mikloso...@netapp.com> wrote:

> One more point as I was reading your email in a hurry. The ultimate goal
> is to run all tests. So even if we apply the proposed regexp in checkstyle
> and there is an unfortunate case that MyTestSplit1 would slip through on a
> review and it would not be executed in CI, if we do not figure out some
> better regexp, we might relax "test.name" property to include Split*.java
> as well. Yes, this might be also done.
>
> Enforcing "all tests to end on "Test" " means that we would need to know
> which .java files contain tests and which not without looking into them. I
> do not think that is possible under circumstances we have where (checkstyle
> module).
>
> 
> From: Derek Chen-Becker 
> Sent: Monday, October 24, 2022 15:53
> 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.
>
>
>
> OK, that makes sense, and I missed the rename of "TestBaseImpl" (which I
> agree is in need of fixing). When you say:
>
> > What we are trying to achieve by that is to have a test which ends on
> "Test" and nothing else and it may contain "Test" only in case it does not
> start with it.
>
> Should the regex enforce the "which ends on Test" part? Otherwise, if we
> want to allow tests like MyTestSplit1, do we need to update the "test.name
> " parameter to allow that in the test runner?
>
> Thanks,
>
> Derek
>
>
> On Mon, Oct 24, 2022 at 7:27 AM Miklosovic, Stefan <
> stefan.mikloso...@netapp.com> wrote:
> Hi Derek,
>
> yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$
>
> What we are trying to achieve by that is to have a test which ends on
> "Test" and nothing else and it may contain "Test" only in case it does not
> start with it.
>
> To your second paragraph, I do not think we have any *Base files which are
> tests (that would be a bug) nor they are standalone (not inherited). By
> first approach, TestBaseImpl (which we do have) would start to be invalid,
> we would have to rename it to "DistributedTestBaseImpl" because it was
> starting on "Test" which is no-no. I agree that "TestBaseImpl" is rather
> unfortunate name. Currently, TestBaseImpl itself extends
> "DistributedTestBase" so in the end we would have "DistributedTestBaseImpl
> -> TestBaseImpl -> DistributedTestBase". Please keep in mind that
> "DistributedTestBase" is located in dtest jar (separate project) and that
> is rather inconvenient to rename.
>
> Regards
>
> 
> From: Derek Chen-Becker  de...@chen-becker.org>>
> Sent: Monday, October 24, 2022 15:09
> 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.
>
>
>
> For clarity, what is the final regex you've landed on for the first
> approach? Is it

Re: [DISCUSS] Potential circleci config and workflow changes

[Josh McKenzie]

> Auto-run on push? Can you elaborate?
Yep - instead of having to go to circle and click, when you push your branch 
the circle hook picks it up and kicks off the top level job automatically. I 
tend to be paranoid and push a lot of incremental work that's not ready for CI 
remotely so it's not great for me, but I think having it be optional is the 
Right Thing.

So here's the outstanding work I've distilled from this thread:
- Create an epic for circleci improvement work (we have a lot of little 
augments to do here; keep it organized and try and avoid redundancy)
- Include CASSANDRA-17600 in epic umbrella  
- Include CASSANDRA-17930 in epic umbrella
- Ticket to tune parallelism per job  
-  
> def java_parallelism(src_dir, kind, num_file_in_worker, include = lambda 
a, b: True):
> d = os.path.join(src_dir, 'test', kind)
> num_files = 0
> for root, dirs, files in os.walk(d):
> for f in files:
> if f.endswith('Test.java') and include(os.path.join(root, f), 
f):
> num_files += 1
> return math.floor(num_files / num_file_in_worker)
> 
> def fix_parallelism(args, contents):
> jobs = contents['jobs']
> 
> unit_parallelism= java_parallelism(args.src, 'unit', 
20)
> jvm_dtest_parallelism   = java_parallelism(args.src, 
'distributed', 4, lambda full, name: 'upgrade' not in full)
> jvm_dtest_upgrade_parallelism   = java_parallelism(args.src, 
'distributed', 2, lambda full, name: 'upgrade' in full)
- `TL;DR - I find all test files we are going to run, and based off a 
pre-defined variable that says “idea” number of files per worker, I then 
calculate how many workers we need.  So unit tests are num_files / 20 ~= 35 
workers.  Can I be “smarter” by knowing which files have higher cost?  Sure… 
but the “perfect” and the “average” are too similar that it wasn’t worth it...` 
 
- Ticket to combine pre-commit jobs into 1 pipeline for all JDK's
- Path to activate all supported JDK's for pre-commit at root (one-click 
pre-merge full validation)
- Path to activate per JDK below that (interim work partial validation)
- Ticket to rename jobs in circleci
- Reference comment: 
https://issues.apache.org/jira/browse/CASSANDRA-17939?focusedCommentId=17617016&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17617016
- (buildjdk)_(runjdk)_(testsuite) format:
- j8_j8_jvm_dtests
- j8_j11_jvm_dtests
- j11_j11_jvm_dtest_vnode
etc
- Ticket for flag in generate.sh to support auto run on push (see response 
above)
- Ticket for: remove -h, have -f and -p (free and paid) (probably intersects 
with https://issues.apache.org/jira/browse/CASSANDRA-17600)

Anything wrong w/the above or anything missed? If not, I'll go do some JIRA'ing.

~Josh


On Fri, Oct 21, 2022, at 3:50 PM, Josh McKenzie wrote:
>> I am cool with removing circle if apache CI is stable and works, we do need 
>> to solve the non-committer issue but would argue that partially exists in 
>> circle today (you can be a non-commuter with a paid account, but you can’t 
>> be a non-committer with a free account)
> There's a few threads here:
> 1. non-committers should be able to run ci
> 2. People that have resources and want to run ci faster should be able to do 
> so (assuming the ci of record could serve to be faster)
> 3. ci should be stable
> 
> Thus far we haven't landed on 1 system that satisfies all 3. There's some 
> background discussions brainstorming how to get there; when / if things come 
> from that they'll as always be brought to the list for discussion.
> 
> On Fri, Oct 21, 2022, at 1:44 PM, Ekaterina Dimitrova wrote:
>> I agree with David with one caveat - last time I checked only some Python 
>> tests lack enough resources with the free tier. The rest run slower than 
>> with a paid account, but they do fine. In fact I use the free tier if I want 
>> to test only unit or in-jvm tests sometimes. I guess that is what he meant 
>> by partially but even being able to run the non-Python tests is a win IMHO. 
>> If we find a solution for all tests though… even better.
>> @Derek your idea sounds interesting, I will be happy to see a proposal. 
>> Thank you
>> 
>> On Fri, 21 Oct 2022 at 13:39, David Capwell  wrote:
>>> I am cool with removing circle if apache CI is stable and works, we do need 
>>> to solve the non-committer issue but would argue that partially exists in 
>>> circle today (you can be a non-commuter with a paid account, but you can’t 
>>> be a non-committer with a free account)
>>> 
>>> 
>>> 
 On Oct 20, 2022, at 2:20 PM, Josh McKenzie  wrote:
 
> I believe it's original intention to be just about CircleCI.
 It was but fwiw I'm good w/us exploring adjacent things regarding CI here. 
 I'm planning on deep diving on the thread tomorrow and distilling a 
 snapshot of the work we have a consensus on for circle and summa

Re: Some tests are never executed in CI due to their name

[Miklosovic, Stefan]

Yeah, that is what the branch in my original email actually already solved. I 
mean ...

CassandraAuthorizerTruncatingTests
BatchTests
UUIDTests

these are ending on Tests which is illegal and they are fixed there.

Other cases are either "TestBase" or "Tester" which should be legal.

I think using the first approach and fixing all "SplitN" PLUS adding Split* to 
test.name regexp should do it.

I think we can "automate" at most like you suggest and scan it manually, fix 
the stuff and from then incorporate checkstyle to take care of that.

There are also some classes which do include @Test methods but I think they are 
abstract as they are meant to be extended as the real test is just wrapping 
that. This might happen when there are slight variations across test classes. 
This is fine as well.


From: Derek Chen-Becker 
Sent: Monday, October 24, 2022 19:18
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.



OK, I think the approach you're proposing is fine. As an orthogonal idea, is 
there any way to do a one-time audit to narrow down files that we need to 
inspect, or any way to automate confirmation of a file containing tests? For 
example:

* Start with all of the files under the `test` directory (is it safe to assume 
that this is the only location for test classes?) => 1457 files
* Remove any files that already end in "Test.java" => down to 396 files
* Is it safe to assume that only files having "Test" somewhere in their name 
are test classes? That reduces the list down to 60 files

Alternatively, are we only talking about unit tests? Would it be sufficient to 
look for files that don't end in "Test.java" but contain "@Test" annotations? 
That's a significantly smaller set, so maybe we could fix unit tests first:

❯❯❯ for fl in test/unit/**/*.java; do if [[ $fl != *Test.java ]]; then if grep 
-qF '@Test' $fl; then print $fl; fi; fi; done   

   circleci-parity ✱ ◼
test/unit/org/apache/cassandra/auth/CassandraAuthorizerTruncatingTests.java
test/unit/org/apache/cassandra/cql3/BatchTests.java
test/unit/org/apache/cassandra/cql3/KeywordTestBase.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexTester.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
test/unit/org/apache/cassandra/dht/PartitionerTestCase.java
test/unit/org/apache/cassandra/utils/UUIDTests.java

Cheers,

Derek


On Mon, Oct 24, 2022 at 9:00 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>> wrote:
One more point as I was reading your email in a hurry. The ultimate goal is to 
run all tests. So even if we apply the proposed regexp in checkstyle and there 
is an unfortunate case that MyTestSplit1 would slip through on a review and it 
would not be executed in CI, if we do not figure out some better regexp, we 
might relax "test.name" property to include Split*.java as 
well. Yes, this might be also done.

Enforcing "all tests to end on "Test" " means that we would need to know which 
.java files contain tests and which not without looking into them. I do not 
think that is possible under circumstances we have where (checkstyle module).


From: Derek Chen-Becker mailto:de...@chen-becker.org>>
Sent: Monday, October 24, 2022 15:53
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.



OK, that makes sense, and I missed the rename of "TestBaseImpl" (which I agree 
is in need of fixing). When you say:

> What we are trying to achieve by that is to have a test which ends on "Test" 
> and nothing else and it may contain "Test" only in case it does not start 
> with it.

Should the regex enforce the "which ends on Test" part? Otherwise, if we want 
to allow tests like MyTestSplit1, do we need to update the 
"test.name" parameter to allow that in the 
test runner?

Thanks,

Derek


On Mon, Oct 24, 2022 at 7:27 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>>>
 wrote:
Hi Derek,

yes, the regexp is ^(?!(?:Test))(?!.*?(?:Tests)).*$

What we are trying to achieve by 

Re: [DISCUSS] Potential circleci config and workflow changes

[Andrés de la Peña]

>
> Yep - instead of having to go to circle and click, when you push your
> branch the circle hook picks it up and kicks off the top level job
> automatically. I tend to be paranoid and push a lot of incremental work
> that's not ready for CI remotely so it's not great for me, but I think
> having it be optional is the Right Thing.

- Ticket for flag in generate.sh to support auto run on push (see response
> above)


CASSANDRA-17113 was created almost a year ago for this. While we can have
flags to specify whether the runs tart automatically or not, we'd still
need to have a default. I think the default should be not starting anything
without either manual approval or the usage of those flags when generating
the config, as we decided during CASSANDRA-16882 and the discussions around
it.

- Ticket to combine pre-commit jobs into 1 pipeline for all JDK's
> - Ticket to rename jobs in circleci


I'd say these two things should be in a single ticket, since the problems
with naming appear when we try to unify the two workflows.


On Mon, 24 Oct 2022 at 19:10, Josh McKenzie  wrote:

> Auto-run on push? Can you elaborate?
>
> Yep - instead of having to go to circle and click, when you push your
> branch the circle hook picks it up and kicks off the top level job
> automatically. I tend to be paranoid and push a lot of incremental work
> that's not ready for CI remotely so it's not great for me, but I think
> having it be optional is the Right Thing.
>
> So here's the outstanding work I've distilled from this thread:
> - Create an epic for circleci improvement work (we have a lot of little
> augments to do here; keep it organized and try and avoid redundancy)
> - Include CASSANDRA-17600 in epic umbrella
> - Include CASSANDRA-17930 in epic umbrella
> - Ticket to tune parallelism per job
> -
> > def java_parallelism(src_dir, kind, num_file_in_worker, include =
> lambda a, b: True):
> > d = os.path.join(src_dir, 'test', kind)
> > num_files = 0
> > for root, dirs, files in os.walk(d):
> > for f in files:
> > if f.endswith('Test.java') and
> include(os.path.join(root, f), f):
> > num_files += 1
> > return math.floor(num_files / num_file_in_worker)
> >
> > def fix_parallelism(args, contents):
> > jobs = contents['jobs']
> >
> > unit_parallelism= java_parallelism(args.src,
> 'unit', 20)
> > jvm_dtest_parallelism   = java_parallelism(args.src,
> 'distributed', 4, lambda full, name: 'upgrade' not in full)
> > jvm_dtest_upgrade_parallelism   = java_parallelism(args.src,
> 'distributed', 2, lambda full, name: 'upgrade' in full)
> - `TL;DR - I find all test files we are going to run, and based off a
> pre-defined variable that says “idea” number of files per worker, I then
> calculate how many workers we need.  So unit tests are num_files / 20 ~= 35
> workers.  Can I be “smarter” by knowing which files have higher cost?
> Sure… but the “perfect” and the “average” are too similar that it wasn’t
> worth it...`
> - Ticket to combine pre-commit jobs into 1 pipeline for all JDK's
> - Path to activate all supported JDK's for pre-commit at root
> (one-click pre-merge full validation)
> - Path to activate per JDK below that (interim work partial validation)
> - Ticket to rename jobs in circleci
> - Reference comment:
> https://issues.apache.org/jira/browse/CASSANDRA-17939?focusedCommentId=17617016&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17617016
> - (buildjdk)_(runjdk)_(testsuite) format:
> - j8_j8_jvm_dtests
> - j8_j11_jvm_dtests
> - j11_j11_jvm_dtest_vnode
> etc
> - Ticket for flag in generate.sh to support auto run on push (see response
> above)
> - Ticket for: remove -h, have -f and -p (free and paid) (probably
> intersects with https://issues.apache.org/jira/browse/CASSANDRA-17600)
>
> Anything wrong w/the above or anything missed? If not, I'll go do some
> JIRA'ing.
>
> ~Josh
>
>
> On Fri, Oct 21, 2022, at 3:50 PM, Josh McKenzie wrote:
>
> I am cool with removing circle if apache CI is stable and works, we do
> need to solve the non-committer issue but would argue that partially exists
> in circle today (you can be a non-commuter with a paid account, but you
> can’t be a non-committer with a free account)
>
> There's a few threads here:
> 1. non-committers should be able to run ci
> 2. People that have resources and want to run ci faster should be able to
> do so (assuming the ci of record could serve to be faster)
> 3. ci should be stable
>
> Thus far we haven't landed on 1 system that satisfies all 3. There's some
> background discussions brainstorming how to get there; when / if things
> come from that they'll as always be brought to the list for discussion.
>
> On Fri, Oct 21, 2022, at 1:44 PM, Ekaterina Dimitrova wrote:
>
> I agree with David with one caveat - last time I checked only some Python

Re: [DISCUSS] Potential circleci config and workflow changes

[Ekaterina Dimitrova]

Thank you Josh

So about push with/without a single click, I guess you mean to parameterize
whether the step build needs approval or not? Pre-commit the new flag will
use the “no-approval” version, but during development we still will be able
to push the tests without immediately starting all tests, right?
- parallelism + -h being removed - just to confirm, that means we will not
use xlarge containers. As David confirmed, this is not needed for all jibs
and it is important as otherwise whoever uses paid account will burn their
credits time faster for very similar duration runs.

CASSANDRA-17930 - I will use the opportunity also to mention that many of
the identified missing jobs in CircleCI will be soon there - Andres is
working on all variations unit tests, I am doing final testing on fixing
the Python upgrade tests (we weren’t using the right parameters and running
way more jobs then we should) and Derek is looking into the rest of the
Python test. I still need to check whether we need something regarding
in-jvm etc, the simulator ones are running only for jdk8 for now,
confirmed. All this should unblock us to be able to do next releases based
on CircleCI as we agreed. Then we move to do some
changes/additions/improvements to Jenkins. And of course, the future
improvements we agreed on.

On Mon, 24 Oct 2022 at 14:10, Josh McKenzie  wrote:

> Auto-run on push? Can you elaborate?
>
> Yep - instead of having to go to circle and click, when you push your
> branch the circle hook picks it up and kicks off the top level job
> automatically. I tend to be paranoid and push a lot of incremental work
> that's not ready for CI remotely so it's not great for me, but I think
> having it be optional is the Right Thing.
>
> So here's the outstanding work I've distilled from this thread:
> - Create an epic for circleci improvement work (we have a lot of little
> augments to do here; keep it organized and try and avoid redundancy)
> - Include CASSANDRA-17600 in epic umbrella
> - Include CASSANDRA-17930 in epic umbrella
> - Ticket to tune parallelism per job
> -
> > def java_parallelism(src_dir, kind, num_file_in_worker, include =
> lambda a, b: True):
> > d = os.path.join(src_dir, 'test', kind)
> > num_files = 0
> > for root, dirs, files in os.walk(d):
> > for f in files:
> > if f.endswith('Test.java') and
> include(os.path.join(root, f), f):
> > num_files += 1
> > return math.floor(num_files / num_file_in_worker)
> >
> > def fix_parallelism(args, contents):
> > jobs = contents['jobs']
> >
> > unit_parallelism= java_parallelism(args.src,
> 'unit', 20)
> > jvm_dtest_parallelism   = java_parallelism(args.src,
> 'distributed', 4, lambda full, name: 'upgrade' not in full)
> > jvm_dtest_upgrade_parallelism   = java_parallelism(args.src,
> 'distributed', 2, lambda full, name: 'upgrade' in full)
> - `TL;DR - I find all test files we are going to run, and based off a
> pre-defined variable that says “idea” number of files per worker, I then
> calculate how many workers we need.  So unit tests are num_files / 20 ~= 35
> workers.  Can I be “smarter” by knowing which files have higher cost?
> Sure… but the “perfect” and the “average” are too similar that it wasn’t
> worth it...`
> - Ticket to combine pre-commit jobs into 1 pipeline for all JDK's
> - Path to activate all supported JDK's for pre-commit at root
> (one-click pre-merge full validation)
> - Path to activate per JDK below that (interim work partial validation)
> - Ticket to rename jobs in circleci
> - Reference comment:
> https://issues.apache.org/jira/browse/CASSANDRA-17939?focusedCommentId=17617016&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17617016
> - (buildjdk)_(runjdk)_(testsuite) format:
> - j8_j8_jvm_dtests
> - j8_j11_jvm_dtests
> - j11_j11_jvm_dtest_vnode
> etc
> - Ticket for flag in generate.sh to support auto run on push (see response
> above)
> - Ticket for: remove -h, have -f and -p (free and paid) (probably
> intersects with https://issues.apache.org/jira/browse/CASSANDRA-17600)
>
> Anything wrong w/the above or anything missed? If not, I'll go do some
> JIRA'ing.
>
>
> ~Josh
>
>
> On Fri, Oct 21, 2022, at 3:50 PM, Josh McKenzie wrote:
>
> I am cool with removing circle if apache CI is stable and works, we do
> need to solve the non-committer issue but would argue that partially exists
> in circle today (you can be a non-commuter with a paid account, but you
> can’t be a non-committer with a free account)
>
> There's a few threads here:
> 1. non-committers should be able to run ci
> 2. People that have resources and want to run ci faster should be able to
> do so (assuming the ci of record could serve to be faster)
> 3. ci should be stable
>
> Thus far we haven't landed on 1 system that satisfies all 3. There's some
> background disc

Re: [DISCUSS] Potential circleci config and workflow changes

[Ekaterina Dimitrova]

Seems like my email crashed with Andres’ one.
My understanding is we will use the ticket CASSANDRA-17113 as placeholder,
the work there will be rebased/reworked etc depending on what we agree
with.
I also agree with the other points he made. Sounds reasonable to me

On Mon, 24 Oct 2022 at 15:03, Ekaterina Dimitrova 
wrote:

> Thank you Josh
>
> So about push with/without a single click, I guess you mean to
> parameterize whether the step build needs approval or not? Pre-commit the
> new flag will use the “no-approval” version, but during development we
> still will be able to push the tests without immediately starting all
> tests, right?
> - parallelism + -h being removed - just to confirm, that means we will not
> use xlarge containers. As David confirmed, this is not needed for all jibs
> and it is important as otherwise whoever uses paid account will burn their
> credits time faster for very similar duration runs.
>
> CASSANDRA-17930 - I will use the opportunity also to mention that many of
> the identified missing jobs in CircleCI will be soon there - Andres is
> working on all variations unit tests, I am doing final testing on fixing
> the Python upgrade tests (we weren’t using the right parameters and running
> way more jobs then we should) and Derek is looking into the rest of the
> Python test. I still need to check whether we need something regarding
> in-jvm etc, the simulator ones are running only for jdk8 for now,
> confirmed. All this should unblock us to be able to do next releases based
> on CircleCI as we agreed. Then we move to do some
> changes/additions/improvements to Jenkins. And of course, the future
> improvements we agreed on.
>
> On Mon, 24 Oct 2022 at 14:10, Josh McKenzie  wrote:
>
>> Auto-run on push? Can you elaborate?
>>
>> Yep - instead of having to go to circle and click, when you push your
>> branch the circle hook picks it up and kicks off the top level job
>> automatically. I tend to be paranoid and push a lot of incremental work
>> that's not ready for CI remotely so it's not great for me, but I think
>> having it be optional is the Right Thing.
>>
>> So here's the outstanding work I've distilled from this thread:
>> - Create an epic for circleci improvement work (we have a lot of little
>> augments to do here; keep it organized and try and avoid redundancy)
>> - Include CASSANDRA-17600 in epic umbrella
>> - Include CASSANDRA-17930 in epic umbrella
>> - Ticket to tune parallelism per job
>> -
>> > def java_parallelism(src_dir, kind, num_file_in_worker, include =
>> lambda a, b: True):
>> > d = os.path.join(src_dir, 'test', kind)
>> > num_files = 0
>> > for root, dirs, files in os.walk(d):
>> > for f in files:
>> > if f.endswith('Test.java') and
>> include(os.path.join(root, f), f):
>> > num_files += 1
>> > return math.floor(num_files / num_file_in_worker)
>> >
>> > def fix_parallelism(args, contents):
>> > jobs = contents['jobs']
>> >
>> > unit_parallelism= java_parallelism(args.src,
>> 'unit', 20)
>> > jvm_dtest_parallelism   = java_parallelism(args.src,
>> 'distributed', 4, lambda full, name: 'upgrade' not in full)
>> > jvm_dtest_upgrade_parallelism   = java_parallelism(args.src,
>> 'distributed', 2, lambda full, name: 'upgrade' in full)
>> - `TL;DR - I find all test files we are going to run, and based off a
>> pre-defined variable that says “idea” number of files per worker, I then
>> calculate how many workers we need.  So unit tests are num_files / 20 ~= 35
>> workers.  Can I be “smarter” by knowing which files have higher cost?
>> Sure… but the “perfect” and the “average” are too similar that it wasn’t
>> worth it...`
>> - Ticket to combine pre-commit jobs into 1 pipeline for all JDK's
>> - Path to activate all supported JDK's for pre-commit at root
>> (one-click pre-merge full validation)
>> - Path to activate per JDK below that (interim work partial
>> validation)
>> - Ticket to rename jobs in circleci
>> - Reference comment:
>> https://issues.apache.org/jira/browse/CASSANDRA-17939?focusedCommentId=17617016&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17617016
>> - (buildjdk)_(runjdk)_(testsuite) format:
>> - j8_j8_jvm_dtests
>> - j8_j11_jvm_dtests
>> - j11_j11_jvm_dtest_vnode
>> etc
>> - Ticket for flag in generate.sh to support auto run on push (see
>> response above)
>> - Ticket for: remove -h, have -f and -p (free and paid) (probably
>> intersects with https://issues.apache.org/jira/browse/CASSANDRA-17600)
>>
>> Anything wrong w/the above or anything missed? If not, I'll go do some
>> JIRA'ing.
>>
>>
>> ~Josh
>>
>>
>> On Fri, Oct 21, 2022, at 3:50 PM, Josh McKenzie wrote:
>>
>> I am cool with removing circle if apache CI is stable and works, we do
>> need to solve the non-committer issue but would argue that partially exists
>> in circ

Re: [DISCUSS] Potential circleci config and workflow changes

[Andrés de la Peña]

>
> - Ticket for: remove -h, have -f and -p (free and paid)


+1 to this, probably there isn't anyone using -h. There are some jobs that
can't pass with the free option. Maybe we should remove them from the
workflow when the free option is used. Perhaps that could save new
contributors some confusion. Or should we leave them because a subset of
the tests inside those jobs can still pass even with the free tier?

By the way, the generate.sh script already accepts a -f flag. It's used to
stop checking that the specified environment variables are known. It was
meant to be a kind of general "--force" flag.

On Mon, 24 Oct 2022 at 20:07, Ekaterina Dimitrova 
wrote:

> Seems like my email crashed with Andres’ one.
> My understanding is we will use the ticket CASSANDRA-17113 as
> placeholder, the work there will be rebased/reworked etc depending on what
> we agree with.
> I also agree with the other points he made. Sounds reasonable to me
>
> On Mon, 24 Oct 2022 at 15:03, Ekaterina Dimitrova 
> wrote:
>
>> Thank you Josh
>>
>> So about push with/without a single click, I guess you mean to
>> parameterize whether the step build needs approval or not? Pre-commit the
>> new flag will use the “no-approval” version, but during development we
>> still will be able to push the tests without immediately starting all
>> tests, right?
>> - parallelism + -h being removed - just to confirm, that means we will
>> not use xlarge containers. As David confirmed, this is not needed for all
>> jibs and it is important as otherwise whoever uses paid account will burn
>> their credits time faster for very similar duration runs.
>>
>> CASSANDRA-17930 - I will use the opportunity also to mention that many of
>> the identified missing jobs in CircleCI will be soon there - Andres is
>> working on all variations unit tests, I am doing final testing on fixing
>> the Python upgrade tests (we weren’t using the right parameters and running
>> way more jobs then we should) and Derek is looking into the rest of the
>> Python test. I still need to check whether we need something regarding
>> in-jvm etc, the simulator ones are running only for jdk8 for now,
>> confirmed. All this should unblock us to be able to do next releases based
>> on CircleCI as we agreed. Then we move to do some
>> changes/additions/improvements to Jenkins. And of course, the future
>> improvements we agreed on.
>>
>> On Mon, 24 Oct 2022 at 14:10, Josh McKenzie  wrote:
>>
>>> Auto-run on push? Can you elaborate?
>>>
>>> Yep - instead of having to go to circle and click, when you push your
>>> branch the circle hook picks it up and kicks off the top level job
>>> automatically. I tend to be paranoid and push a lot of incremental work
>>> that's not ready for CI remotely so it's not great for me, but I think
>>> having it be optional is the Right Thing.
>>>
>>> So here's the outstanding work I've distilled from this thread:
>>> - Create an epic for circleci improvement work (we have a lot of little
>>> augments to do here; keep it organized and try and avoid redundancy)
>>> - Include CASSANDRA-17600 in epic umbrella
>>> - Include CASSANDRA-17930 in epic umbrella
>>> - Ticket to tune parallelism per job
>>> -
>>> > def java_parallelism(src_dir, kind, num_file_in_worker, include =
>>> lambda a, b: True):
>>> > d = os.path.join(src_dir, 'test', kind)
>>> > num_files = 0
>>> > for root, dirs, files in os.walk(d):
>>> > for f in files:
>>> > if f.endswith('Test.java') and
>>> include(os.path.join(root, f), f):
>>> > num_files += 1
>>> > return math.floor(num_files / num_file_in_worker)
>>> >
>>> > def fix_parallelism(args, contents):
>>> > jobs = contents['jobs']
>>> >
>>> > unit_parallelism= java_parallelism(args.src,
>>> 'unit', 20)
>>> > jvm_dtest_parallelism   = java_parallelism(args.src,
>>> 'distributed', 4, lambda full, name: 'upgrade' not in full)
>>> > jvm_dtest_upgrade_parallelism   = java_parallelism(args.src,
>>> 'distributed', 2, lambda full, name: 'upgrade' in full)
>>> - `TL;DR - I find all test files we are going to run, and based off
>>> a pre-defined variable that says “idea” number of files per worker, I then
>>> calculate how many workers we need.  So unit tests are num_files / 20 ~= 35
>>> workers.  Can I be “smarter” by knowing which files have higher cost?
>>> Sure… but the “perfect” and the “average” are too similar that it wasn’t
>>> worth it...`
>>> - Ticket to combine pre-commit jobs into 1 pipeline for all JDK's
>>> - Path to activate all supported JDK's for pre-commit at root
>>> (one-click pre-merge full validation)
>>> - Path to activate per JDK below that (interim work partial
>>> validation)
>>> - Ticket to rename jobs in circleci
>>> - Reference comment:
>>> https://issues.apache.org/jira/browse/CASSANDRA-17939?focusedCommentId=17617016&page=com.atlassian.jira.plugin.system.is

Re: [DISCUSS] Potential circleci config and workflow changes

[Josh McKenzie]

@Ekaterina: I recall us going back and forth on whether default should be 
require approval or not and there not being a consensus. I'm fine not changing 
the status quo and just parameterizing that in generate.sh so folks can locally 
script how they want to setup when they alias up generate.sh.

I'll add C-17113 to the epic as well and any other tickets anyone has in flight 
we can link up.

> Maybe we should remove them from the workflow when the free option is used
That'd put us in the position of having a "smoke testing suite" for free tier 
users and the expectation of a committer running the full suite pre-merge. 
Which, now that I type it out, is a lot more representative of our current 
reality so we should probably do that.

Noted re: the -f flag; I could have checked that but just hacked that out in 
the email spur of the moment. We could just default to low / free / smoke test 
and have -p for paid tier.


On Mon, Oct 24, 2022, at 3:23 PM, Andrés de la Peña wrote:
>> - Ticket for: remove -h, have -f and -p (free and paid)
> 
> +1 to this, probably there isn't anyone using -h. There are some jobs that 
> can't pass with the free option. Maybe we should remove them from the 
> workflow when the free option is used. Perhaps that could save new 
> contributors some confusion. Or should we leave them because a subset of the 
> tests inside those jobs can still pass even with the free tier?
> 
> By the way, the generate.sh script already accepts a -f flag. It's used to 
> stop checking that the specified environment variables are known. It was 
> meant to be a kind of general "--force" flag.
> 
> On Mon, 24 Oct 2022 at 20:07, Ekaterina Dimitrova  
> wrote:
>> Seems like my email crashed with Andres’ one. 
>> My understanding is we will use the ticket CASSANDRA-17113 as placeholder, 
>> the work there will be rebased/reworked etc depending on what we agree with. 
>> I also agree with the other points he made. Sounds reasonable to me
>> 
>> On Mon, 24 Oct 2022 at 15:03, Ekaterina Dimitrova  
>> wrote:
>>> Thank you Josh
>>> 
>>> So about push with/without a single click, I guess you mean to parameterize 
>>> whether the step build needs approval or not? Pre-commit the new flag will 
>>> use the “no-approval” version, but during development we still will be able 
>>> to push the tests without immediately starting all tests, right?
>>> - parallelism + -h being removed - just to confirm, that means we will not 
>>> use xlarge containers. As David confirmed, this is not needed for all jibs 
>>> and it is important as otherwise whoever uses paid account will burn their 
>>> credits time faster for very similar duration runs. 
>>> 
>>> CASSANDRA-17930 - I will use the opportunity also to mention that many of 
>>> the identified missing jobs in CircleCI will be soon there - Andres is 
>>> working on all variations unit tests, I am doing final testing on fixing 
>>> the Python upgrade tests (we weren’t using the right parameters and running 
>>> way more jobs then we should) and Derek is looking into the rest of the 
>>> Python test. I still need to check whether we need something regarding 
>>> in-jvm etc, the simulator ones are running only for jdk8 for now, 
>>> confirmed. All this should unblock us to be able to do next releases based 
>>> on CircleCI as we agreed. Then we move to do some 
>>> changes/additions/improvements to Jenkins. And of course, the future 
>>> improvements we agreed on. 
>>> 
>>> On Mon, 24 Oct 2022 at 14:10, Josh McKenzie  wrote:
 __
> Auto-run on push? Can you elaborate?
 Yep - instead of having to go to circle and click, when you push your 
 branch the circle hook picks it up and kicks off the top level job 
 automatically. I tend to be paranoid and push a lot of incremental work 
 that's not ready for CI remotely so it's not great for me, but I think 
 having it be optional is the Right Thing.
 
 So here's the outstanding work I've distilled from this thread:
 - Create an epic for circleci improvement work (we have a lot of little 
 augments to do here; keep it organized and try and avoid redundancy)
 - Include CASSANDRA-17600 in epic umbrella  
 - Include CASSANDRA-17930 in epic umbrella
 - Ticket to tune parallelism per job  
 -  
 > def java_parallelism(src_dir, kind, num_file_in_worker, include = 
 lambda a, b: True):
 > d = os.path.join(src_dir, 'test', kind)
 > num_files = 0
 > for root, dirs, files in os.walk(d):
 > for f in files:
 > if f.endswith('Test.java') and 
 include(os.path.join(root, f), f):
 > num_files += 1
 > return math.floor(num_files / num_file_in_worker)
 > 
 > def fix_parallelism(args, contents):
 > jobs = contents['jobs']
 > 
 > unit_parallelism= java_parallelism(args.src, 
 'unit', 20)
 > j

Re: [DISCUSS] Potential circleci config and workflow changes

[Derek Chen-Becker]

This could also be a pipeline parameter instead of hacking it in
generate.sh. I promise I'll have a proposal before the end of the week.

Derek

On Mon, Oct 24, 2022 at 2:13 PM Josh McKenzie  wrote:

> @Ekaterina: I recall us going back and forth on whether default should be
> require approval or not and there not being a consensus. I'm fine not
> changing the status quo and just parameterizing that in generate.sh so
> folks can locally script how they want to setup when they alias up
> generate.sh.
>
> I'll add C-17113 to the epic as well and any other tickets anyone has in
> flight we can link up.
>
> Maybe we should remove them from the workflow when the free option is used
>
> That'd put us in the position of having a "smoke testing suite" for free
> tier users and the expectation of a committer running the full suite
> pre-merge. Which, now that I type it out, is a lot more representative of
> our current reality so we should probably do that.
>
> Noted re: the -f flag; I could have checked that but just hacked that out
> in the email spur of the moment. We could just default to low / free /
> smoke test and have -p for paid tier.
>
>
> On Mon, Oct 24, 2022, at 3:23 PM, Andrés de la Peña wrote:
>
> - Ticket for: remove -h, have -f and -p (free and paid)
>
>
> +1 to this, probably there isn't anyone using -h. There are some jobs that
> can't pass with the free option. Maybe we should remove them from the
> workflow when the free option is used. Perhaps that could save new
> contributors some confusion. Or should we leave them because a subset of
> the tests inside those jobs can still pass even with the free tier?
>
> By the way, the generate.sh script already accepts a -f flag. It's used to
> stop checking that the specified environment variables are known. It was
> meant to be a kind of general "--force" flag.
>
> On Mon, 24 Oct 2022 at 20:07, Ekaterina Dimitrova 
> wrote:
>
> Seems like my email crashed with Andres’ one.
> My understanding is we will use the ticket CASSANDRA-17113 as
> placeholder, the work there will be rebased/reworked etc depending on what
> we agree with.
> I also agree with the other points he made. Sounds reasonable to me
>
> On Mon, 24 Oct 2022 at 15:03, Ekaterina Dimitrova 
> wrote:
>
> Thank you Josh
>
> So about push with/without a single click, I guess you mean to
> parameterize whether the step build needs approval or not? Pre-commit the
> new flag will use the “no-approval” version, but during development we
> still will be able to push the tests without immediately starting all
> tests, right?
> - parallelism + -h being removed - just to confirm, that means we will not
> use xlarge containers. As David confirmed, this is not needed for all jibs
> and it is important as otherwise whoever uses paid account will burn their
> credits time faster for very similar duration runs.
>
> CASSANDRA-17930 - I will use the opportunity also to mention that many of
> the identified missing jobs in CircleCI will be soon there - Andres is
> working on all variations unit tests, I am doing final testing on fixing
> the Python upgrade tests (we weren’t using the right parameters and running
> way more jobs then we should) and Derek is looking into the rest of the
> Python test. I still need to check whether we need something regarding
> in-jvm etc, the simulator ones are running only for jdk8 for now,
> confirmed. All this should unblock us to be able to do next releases based
> on CircleCI as we agreed. Then we move to do some
> changes/additions/improvements to Jenkins. And of course, the future
> improvements we agreed on.
>
> On Mon, 24 Oct 2022 at 14:10, Josh McKenzie  wrote:
>
>
> Auto-run on push? Can you elaborate?
>
> Yep - instead of having to go to circle and click, when you push your
> branch the circle hook picks it up and kicks off the top level job
> automatically. I tend to be paranoid and push a lot of incremental work
> that's not ready for CI remotely so it's not great for me, but I think
> having it be optional is the Right Thing.
>
> So here's the outstanding work I've distilled from this thread:
> - Create an epic for circleci improvement work (we have a lot of little
> augments to do here; keep it organized and try and avoid redundancy)
> - Include CASSANDRA-17600 in epic umbrella
> - Include CASSANDRA-17930 in epic umbrella
> - Ticket to tune parallelism per job
> -
> > def java_parallelism(src_dir, kind, num_file_in_worker, include =
> lambda a, b: True):
> > d = os.path.join(src_dir, 'test', kind)
> > num_files = 0
> > for root, dirs, files in os.walk(d):
> > for f in files:
> > if f.endswith('Test.java') and
> include(os.path.join(root, f), f):
> > num_files += 1
> > return math.floor(num_files / num_file_in_worker)
> >
> > def fix_parallelism(args, contents):
> > jobs = contents['jobs']
> >
> > unit_parallelism= java_parallelism(ar

Re: Some tests are never executed in CI due to their name

[Berenguer Blasi]

The problem with using the first approach is that it fixes the current 
situation but it doesn't prevent it from happening again. The second 
proposal prevents it from happening again but at the cost of a bigger 
rename I'd volunteer to if needed.


Regards

On 24/10/22 20:38, Miklosovic, Stefan wrote:

Yeah, that is what the branch in my original email actually already solved. I 
mean ...

CassandraAuthorizerTruncatingTests
BatchTests
UUIDTests

these are ending on Tests which is illegal and they are fixed there.

Other cases are either "TestBase" or "Tester" which should be legal.

I think using the first approach and fixing all "SplitN" PLUS adding Split* to 
test.name regexp should do it.

I think we can "automate" at most like you suggest and scan it manually, fix 
the stuff and from then incorporate checkstyle to take care of that.

There are also some classes which do include @Test methods but I think they are 
abstract as they are meant to be extended as the real test is just wrapping 
that. This might happen when there are slight variations across test classes. 
This is fine as well.


From: Derek Chen-Becker 
Sent: Monday, October 24, 2022 19:18
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.



OK, I think the approach you're proposing is fine. As an orthogonal idea, is 
there any way to do a one-time audit to narrow down files that we need to 
inspect, or any way to automate confirmation of a file containing tests? For 
example:

* Start with all of the files under the `test` directory (is it safe to assume 
that this is the only location for test classes?) => 1457 files
* Remove any files that already end in "Test.java" => down to 396 files
* Is it safe to assume that only files having "Test" somewhere in their name 
are test classes? That reduces the list down to 60 files

Alternatively, are we only talking about unit tests? Would it be sufficient to look for files that 
don't end in "Test.java" but contain "@Test" annotations? That's a 
significantly smaller set, so maybe we could fix unit tests first:

❯❯❯ for fl in test/unit/**/*.java; do if [[ $fl != *Test.java ]]; then if grep 
-qF '@Test' $fl; then print $fl; fi; fi; done   

   circleci-parity ✱ ◼
test/unit/org/apache/cassandra/auth/CassandraAuthorizerTruncatingTests.java
test/unit/org/apache/cassandra/cql3/BatchTests.java
test/unit/org/apache/cassandra/cql3/KeywordTestBase.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexTester.java
test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
test/unit/org/apache/cassandra/dht/PartitionerTestCase.java
test/unit/org/apache/cassandra/utils/UUIDTests.java

Cheers,

Derek


On Mon, Oct 24, 2022 at 9:00 AM Miklosovic, Stefan 
mailto:stefan.mikloso...@netapp.com>> wrote:
One more point as I was reading your email in a hurry. The ultimate goal is to run all tests. 
So even if we apply the proposed regexp in checkstyle and there is an unfortunate case that 
MyTestSplit1 would slip through on a review and it would not be executed in CI, if we do not 
figure out some better regexp, we might relax "test.name" 
property to include Split*.java as well. Yes, this might be also done.

Enforcing "all tests to end on "Test" " means that we would need to know which 
.java files contain tests and which not without looking into them. I do not think that is possible 
under circumstances we have where (checkstyle module).


From: Derek Chen-Becker mailto:de...@chen-becker.org>>
Sent: Monday, October 24, 2022 15:53
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.



OK, that makes sense, and I missed the rename of "TestBaseImpl" (which I agree 
is in need of fixing). When you say:


What we are trying to achieve by that is to have a test which ends on "Test" and nothing 
else and it may contain "Test" only in case it does not start with it.

Should the regex enforce the "which ends on Test" part? Otherwise, if we want to allow tests like 
MyTestSplit1, do we need to update the "test.name" 
parameter to allow that in the tes