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

2022-10-25 Thread Miklosovic, Stefan
I think that was already communicated and understood.


From: Berenguer Blasi 
Sent: Tuesday, October 25, 2022 7:26
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.




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 attachmen

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

2022-10-25 Thread Miklosovic, Stefan
I think that what you wrote is not entirely correct. It will prevent it from 
happening again when there are tests ending on "Tests" or starting on "Test". 
The only case it will not cover is "SplitN" issue we plan to cover with relaxed 
test.name property.

It seems like what you wrote means that we will fix it and tests will leak 
again. That is not true.


From: Berenguer Blasi 
Sent: Tuesday, October 25, 2022 7:26
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.




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

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

2022-10-25 Thread Berenguer Blasi

IIUC we're relying on catching the word 'Split' in the file name for option 1. 
If somebody named his test i.e. 'MyTestGroup1', 'MyTestGroup2', 
'TTLTestPre2038', 'TTLTestPost2038',... I think we would leak tests again? or 
any other word that is not specifically accounted for. Unless I am missing sthg 
ofc! :-)

On 25/10/22 10:39, Miklosovic, Stefan wrote:

I think that what you wrote is not entirely correct. It will prevent it from happening again when there are 
tests ending on "Tests" or starting on "Test". The only case it will not cover is 
"SplitN" issue we plan to cover with relaxed test.name property.

It seems like what you wrote means that we will fix it and tests will leak 
again. That is not true.


From: Berenguer Blasi 
Sent: Tuesday, October 25, 2022 7:26
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.




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 
.jav

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

2022-10-25 Thread Miklosovic, Stefan
Hi Berenguer,

I am glad you asked. I was expecting this question.

I think there is a fundamental difference in how we approach this problem. I do 
not say mine is better, I just find it important to describe it clearly.

Let's say we are on a ship which leaks. When I detect that, my course of action 
is to fix the leakage with the biggest patches I can find around with minimal 
effort necessary so we are not taking water anymore. There might still be 
occasional leakages which are very rare but I have a crew who is checking the 
ship constantly anyway (review) and the risk that big leakages like we just 
fixed happen again are relatively very low.

You spot a leakage and instead of fixing it with big patches and calling it a 
day, you are trying to remodel the cabins so they are completely waterproof but 
while doing so, you renamed them so everyone needs to get used to how these 
cabins are called and where they are located, because there is this minimal 
chance that some cadet comes around and starts to live in a cabin he is not 
supposed to and we need to explain it to him. Thousands of cadets found their 
cabins just fine. Occasionally, there are few people who just miss the right 
board completely (Test at start, Tests at the end) and they are automatically 
navigated around (checkstyle) but once in five years there is this guy who just 
completely missed it.

I believe we can just navigate him when that happens. You want to cover that 
guy too.

I just find my approach easier.

You can remodel it all, for sure but I am afraid I will not be a part of that. 
I just do not find it necessary to do that.


From: Berenguer Blasi 
Sent: Tuesday, October 25, 2022 11:08
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.




IIUC we're relying on catching the word 'Split' in the file name for option 1. 
If somebody named his test i.e. 'MyTestGroup1', 'MyTestGroup2', 
'TTLTestPre2038', 'TTLTestPost2038',... I think we would leak tests again? or 
any other word that is not specifically accounted for. Unless I am missing sthg 
ofc! :-)

On 25/10/22 10:39, Miklosovic, Stefan wrote:
> I think that what you wrote is not entirely correct. It will prevent it from 
> happening again when there are tests ending on "Tests" or starting on "Test". 
> The only case it will not cover is "SplitN" issue we plan to cover with 
> relaxed test.name property.
>
> It seems like what you wrote means that we will fix it and tests will leak 
> again. That is not true.
>
> 
> From: Berenguer Blasi 
> Sent: Tuesday, October 25, 2022 7:26
> 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.
>
>
>
>
> 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 

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

2022-10-25 Thread Andrés de la Peña
Note that the test multiplexer also searches for tests ending with "Test",
so it will also miss new or modified tests with nonstandard names. The
automatically detected tests are listed when running generate.sh, and they
are added to the repeated run jobs. That gives us another opportunity to
detect missed tests when we introduce something like "MyTestSplit1.java"
and see that it's not been included in the mandatory repeated runs.

On Tue, 25 Oct 2022 at 10:31, Miklosovic, Stefan <
stefan.mikloso...@netapp.com> wrote:

> Hi Berenguer,
>
> I am glad you asked. I was expecting this question.
>
> I think there is a fundamental difference in how we approach this problem.
> I do not say mine is better, I just find it important to describe it
> clearly.
>
> Let's say we are on a ship which leaks. When I detect that, my course of
> action is to fix the leakage with the biggest patches I can find around
> with minimal effort necessary so we are not taking water anymore. There
> might still be occasional leakages which are very rare but I have a crew
> who is checking the ship constantly anyway (review) and the risk that big
> leakages like we just fixed happen again are relatively very low.
>
> You spot a leakage and instead of fixing it with big patches and calling
> it a day, you are trying to remodel the cabins so they are completely
> waterproof but while doing so, you renamed them so everyone needs to get
> used to how these cabins are called and where they are located, because
> there is this minimal chance that some cadet comes around and starts to
> live in a cabin he is not supposed to and we need to explain it to him.
> Thousands of cadets found their cabins just fine. Occasionally, there are
> few people who just miss the right board completely (Test at start, Tests
> at the end) and they are automatically navigated around (checkstyle) but
> once in five years there is this guy who just completely missed it.
>
> I believe we can just navigate him when that happens. You want to cover
> that guy too.
>
> I just find my approach easier.
>
> You can remodel it all, for sure but I am afraid I will not be a part of
> that. I just do not find it necessary to do that.
>
> 
> From: Berenguer Blasi 
> Sent: Tuesday, October 25, 2022 11:08
> 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.
>
>
>
>
> IIUC we're relying on catching the word 'Split' in the file name for
> option 1. If somebody named his test i.e. 'MyTestGroup1', 'MyTestGroup2',
> 'TTLTestPre2038', 'TTLTestPost2038',... I think we would leak tests again?
> or any other word that is not specifically accounted for. Unless I am
> missing sthg ofc! :-)
>
> On 25/10/22 10:39, Miklosovic, Stefan wrote:
> > I think that what you wrote is not entirely correct. It will prevent it
> from happening again when there are tests ending on "Tests" or starting on
> "Test". The only case it will not cover is "SplitN" issue we plan to cover
> with relaxed test.name property.
> >
> > It seems like what you wrote means that we will fix it and tests will
> leak again. That is not true.
> >
> > 
> > From: Berenguer Blasi 
> > Sent: Tuesday, October 25, 2022 7:26
> > 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.
> >
> >
> >
> >
> > 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

CEP-23: Enhancement for Sparse Data Serialization

2022-10-25 Thread Claude Warren, Jr via dev
I would like to discard CEP-23.  As I am the proposer, is a vote required?

What is the process?

Claude


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

2022-10-25 Thread Berenguer Blasi
Well examples lend themselves to many things. I am sure I could come up 
with examples in the opposite direction. I'd rather talk in terms of the 
actual files and tests we have.


What I was trying to clarify is what I was misunderstanding as I had the 
impression now we were saying option 1 would not leak again.


So option 2, given ant and CI seem to rely on the *Test.java notation 
and also prevents future leaks doesn't seem to me a bad idea. Renaming 
the offending files seems a reasonable cost to me (Eclipse does it for me).


My 2cts! But hey, I've been known to be wrong before :shrug: lol

On 25/10/22 12:04, Andrés de la Peña wrote:
Note that the test multiplexer also searches for tests ending with 
"Test", so it will also miss new or modified tests with nonstandard 
names. The automatically detected tests are listed when running 
generate.sh, and they are added to the repeated run jobs. That gives 
us another opportunity to detect missed tests when we introduce 
something like "MyTestSplit1.java" and see that it's not been included 
in the mandatory repeated runs.


On Tue, 25 Oct 2022 at 10:31, Miklosovic, Stefan 
 wrote:


Hi Berenguer,

I am glad you asked. I was expecting this question.

I think there is a fundamental difference in how we approach this
problem. I do not say mine is better, I just find it important to
describe it clearly.

Let's say we are on a ship which leaks. When I detect that, my
course of action is to fix the leakage with the biggest patches I
can find around with minimal effort necessary so we are not taking
water anymore. There might still be occasional leakages which are
very rare but I have a crew who is checking the ship constantly
anyway (review) and the risk that big leakages like we just fixed
happen again are relatively very low.

You spot a leakage and instead of fixing it with big patches and
calling it a day, you are trying to remodel the cabins so they are
completely waterproof but while doing so, you renamed them so
everyone needs to get used to how these cabins are called and
where they are located, because there is this minimal chance that
some cadet comes around and starts to live in a cabin he is not
supposed to and we need to explain it to him. Thousands of cadets
found their cabins just fine. Occasionally, there are few people
who just miss the right board completely (Test at start, Tests at
the end) and they are automatically navigated around (checkstyle)
but once in five years there is this guy who just completely
missed it.

I believe we can just navigate him when that happens. You want to
cover that guy too.

I just find my approach easier.

You can remodel it all, for sure but I am afraid I will not be a
part of that. I just do not find it necessary to do that.


From: Berenguer Blasi 
Sent: Tuesday, October 25, 2022 11:08
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.




IIUC we're relying on catching the word 'Split' in the file name
for option 1. If somebody named his test i.e. 'MyTestGroup1',
'MyTestGroup2', 'TTLTestPre2038', 'TTLTestPost2038',... I think we
would leak tests again? or any other word that is not specifically
accounted for. Unless I am missing sthg ofc! :-)

On 25/10/22 10:39, Miklosovic, Stefan wrote:
> I think that what you wrote is not entirely correct. It will
prevent it from happening again when there are tests ending on
"Tests" or starting on "Test". The only case it will not cover is
"SplitN" issue we plan to cover with relaxed test.name
 property.
>
> It seems like what you wrote means that we will fix it and tests
will leak again. That is not true.
>
> 
> From: Berenguer Blasi 
> Sent: Tuesday, October 25, 2022 7:26
> 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.
>
>
>
>
> 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
 

Re: CEP-23: Enhancement for Sparse Data Serialization

2022-10-25 Thread Josh McKenzie
... I don't know that we've navigate that question before. My immediate 
reaction is as the proposer you should be able to close it down unless it's 
gone to a vote and/or a vote has passed.

If someone else wants to pick it up later that's fine.

On Tue, Oct 25, 2022, at 7:35 AM, Claude Warren, Jr via dev wrote:
> I would like to discard CEP-23.  As I am the proposer, is a vote required?
> 
> What is the process?
> 
> Claude


Re: CEP-23: Enhancement for Sparse Data Serialization

2022-10-25 Thread Claude Warren, Jr via dev
I see that there is one proposal that was discarded.  I wonder how that got
there.

On Tue, Oct 25, 2022 at 2:52 PM Josh McKenzie  wrote:

> ... I don't know that we've navigate that question before. My immediate
> reaction is as the proposer you should be able to close it down unless it's
> gone to a vote and/or a vote has passed.
>
> If someone else wants to pick it up later that's fine.
>
> On Tue, Oct 25, 2022, at 7:35 AM, Claude Warren, Jr via dev wrote:
>
> I would like to discard CEP-23.  As I am the proposer, is a vote required?
>
> What is the process?
>
> Claude
>
>
>


Re: [DISCUSS] Potential circleci config and workflow changes

2022-10-25 Thread David Capwell
> This could also be a pipeline parameter instead of hacking it in generate.sh


Curious how this works… I run a script that deletes all the approvals and 
removes the testing workflows… I really don’t want to use the UI at all….  I 
assumed pipeline params are a UI thing, but I think the goal here for many of 
us are to ignore the UI other than looking at the results… and even that can be 
scripted...


> On Oct 24, 2022, at 4:44 PM, Derek Chen-Becker  wrote:
> 
> 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  
>> 

Re: [DISCUSS] Potential circleci config and workflow changes

2022-10-25 Thread Derek Chen-Becker
I'm writing up a more complete proposal, but here are some examples.
Parameters can be set from either the UI (not my intent) or via the
circleci CLI. Effectively, the config-2_1.yml can have parameters specified
like:

parameters:
  run_extra_test:
type: boolean
default: false


jobs:
  extra_test:
steps:
  when:
condition: << pipeline.parameters.run_extra_test >>
steps:
  - run : # something
  - run : # something

And then jobs can conditionally execute or not. The "when" clause can also
be applied at the workflows level. When you generate the configuration, you
pass a 2nd yaml file (or text) to the circleci CLI and it will override any
parameters with what you provide. For example, if I wanted to disable the
test:

circleci config process --pipeline-parameters "run_extra_test: false"
config-2_1.yml

The tradeoff is that the config will be more verbose with conditionals. My
first proposal is actually to just use parameters to get rid of the patch
files, since the current patch files simply change values. In this case,
parameters allow us to use the config yaml like a template, which is less
error prone than patch files. However, things like conditional execution
and matrix jobs might allow us to streamline the config, especially as we
support more JDK versions.

Cheers,

Derek

On Tue, Oct 25, 2022 at 10:40 AM David Capwell  wrote:

> This could also be a pipeline parameter instead of hacking it in
> generate.sh
>
>
> Curious how this works… I run a script that deletes all the approvals and
> removes the testing workflows… I really don’t want to use the UI at all….
> I assumed pipeline params are a UI thing, but I think the goal here for
> many of us are to ignore the UI other than looking at the results… and even
> that can be scripted...
>
>
> On Oct 24, 2022, at 4:44 PM, Derek Chen-Becker 
> wrote:
>
> 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 variatio