You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "dcapwell (via GitHub)" <gi...@apache.org> on 2025/02/03 23:25:06 UTC
[PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
dcapwell opened a new pull request, #3865:
URL: https://github.com/apache/cassandra/pull/3865
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell closed pull request #3865: CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors
URL: https://github.com/apache/cassandra/pull/3865
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3865:
URL: https://github.com/apache/cassandra/pull/3865#discussion_r1940273989
##########
test/unit/org/apache/cassandra/tcm/ClusterMetadataSerializerTest.java:
##########
@@ -47,16 +47,16 @@ public class ClusterMetadataSerializerTest
public void serdeLatest()
{
DataOutputBuffer output = new DataOutputBuffer();
- qt().forAll(new ClusterMetadataBuilder().build()).checkAssert(orFail(cm -> {
+ qt().forAll(Generators.toGen(new ClusterMetadataBuilder().build())).check(cm -> {
AsymmetricMetadataSerializers.testSerde(output, ClusterMetadata.serializer, cm, NodeVersion.CURRENT_METADATA_VERSION);
- }));
+ });
}
@Test
public void serdeWithoutAccord()
{
DataOutputBuffer output = new DataOutputBuffer();
- Gen<ClusterMetadata> gen = new ClusterMetadataBuilder().build().assuming(cm -> {
+ Gen<ClusterMetadata> gen = Generators.toGen(new ClusterMetadataBuilder().build()).filter(cm -> {
Review Comment:
accord `filter` doesn't have a bounded amount of retries but `accuming` with QuickTheories does, making this test flakey. The issue is when a CM is generated without anything accord related... we filter those out... if you get unlucky and do this many times in a row `assuming` would reject the build; `filter` does not
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on PR #3865:
URL: https://github.com/apache/cassandra/pull/3865#issuecomment-2641328320
merged https://github.com/apache/cassandra/commit/3089226776b47b2b0420faafbea2b545c0eefff7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3865:
URL: https://github.com/apache/cassandra/pull/3865#discussion_r1940274951
##########
test/unit/org/apache/cassandra/utils/CassandraGenerators.java:
##########
@@ -482,7 +482,12 @@ public TableParamsBuilder withFastPathStrategy()
int size = SourceDSL.integers().between(1, Integer.MAX_VALUE).generate(rnd);
map.put(ParameterizedFastPathStrategy.SIZE, Integer.toString(size));
Set<String> names = new HashSet<>();
- Gen<String> nameGen = SourceDSL.strings().allPossible().ofLengthBetween(1, 10).assuming(s -> !s.trim().isEmpty());
+ Gen<String> nameGen = SourceDSL.strings().allPossible().ofLengthBetween(1, 10)
+ // If : is in the name then the parser will fail; we have validation to disalow this
+ .map(s -> s.replace(":", "_"))
+ // Names are used for DCs and those are seperated by ,
+ .map(s -> s.replace(",", "_"))
Review Comment:
these are the real bug fixes.
`:` caused `dc:weight` parser to fail (we expect 2, but had 3)
`,` caused issues as we had a dc `a,b`
These are generator bugs and not allowed from CQL layer... so switched to `_` to avoid shrinking the size
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3865:
URL: https://github.com/apache/cassandra/pull/3865#discussion_r1940273211
##########
test/unit/org/apache/cassandra/tcm/ClusterMetadataSerializerTest.java:
##########
@@ -47,16 +47,16 @@ public class ClusterMetadataSerializerTest
public void serdeLatest()
{
DataOutputBuffer output = new DataOutputBuffer();
- qt().forAll(new ClusterMetadataBuilder().build()).checkAssert(orFail(cm -> {
+ qt().forAll(Generators.toGen(new ClusterMetadataBuilder().build())).check(cm -> {
Review Comment:
accord `qt` will show the seed that failed when generators fail, but QuickTheories doesn't; making it hard to reproduce these type of errors.
I plan to migrate all tests off of QuickTheories *after* we merge to trunk, and these are cep-15-accord only tests....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3865:
URL: https://github.com/apache/cassandra/pull/3865#discussion_r1940272420
##########
src/java/org/apache/cassandra/service/accord/fastpath/ParameterizedFastPathStrategy.java:
##########
@@ -146,7 +146,7 @@ static WeightedDc fromString(String s, int idx)
else if (parts.length == 2)
return new WeightedDc(validateDC(parts[0]), validateWeight(parts[1]), false);
else
- throw cfe("Invalid dc weighting syntax %s, use <dc>:<weight>");
+ throw cfe("Invalid dc weighting syntax %s, use <dc>:<weight>", s);
Review Comment:
this was the error we saw in UI, and its because `foo:bar:42`... the `dc` had `:` in the name, so hit this logic, but since `, s` was missing it threw an issue with string formatting rather than the CFE we expected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-20286: Accord: TableParamsTest is flakey due to bad generators and production validation logic missed the argument to String.format causing confusing errors [cassandra]
Posted by "dcapwell (via GitHub)" <gi...@apache.org>.
dcapwell commented on code in PR #3865:
URL: https://github.com/apache/cassandra/pull/3865#discussion_r1940273004
##########
test/unit/org/apache/cassandra/schema/TableParamsTest.java:
##########
@@ -36,17 +36,17 @@ public class TableParamsTest
public void serdeLatest()
{
DataOutputBuffer output = new DataOutputBuffer();
- qt().forAll(tableParams()).checkAssert(FailingConsumer.orFail(params -> {
+ qt().forAll(tableParams()).check(params -> {
Review Comment:
accord `qt` will show the seed that failed when generators fail, but QuickTheories doesn't; making it hard to reproduce these type of errors.
I plan to migrate all tests off of QuickTheories *after* we merge to trunk, and these are cep-15-accord only tests....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org