Skip to content

Conversation

@dcapwell
Copy link
Contributor

No description provided.

@dcapwell dcapwell requested a review from ifesdjeen July 25, 2024 17:29
Comment on lines +541 to +542
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for consistency... having this test do seeds differently than the other tests makes it harder for people to jump in and see what's going on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error is consistent for other tests, so migrating to that handling... right now the seed is lost most of the time, so this makes sure it is captured 100% of the time

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is lost in CI....

build.xml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@ifesdjeen ifesdjeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, feel free to ignore. +1 LGTM!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we turn into a suppressed error as above here, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suppressed error

why would we want to suppress? Something unexpected happened and we have no clue how to handle... so we should fail the test. This might be the AssertError we now generate, but we have to push that up... but what if its a different error?

…ose all history in CI

patch by David Capwell; reviewed by Alex Petrov for CASSANDRA-19802
@dcapwell dcapwell merged commit 7f246d5 into apache:trunk Jul 25, 2024
@dcapwell dcapwell deleted the CASSANDRA-19802 branch July 25, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants