Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

roachtest: ignore flaky activerecord tests #135907

Merged
merged 1 commit into from
Nov 23, 2024

Conversation

annrpom
Copy link
Contributor

@annrpom annrpom commented Nov 21, 2024

Fixes: #134811
Fixes: #135801

Release note: None

@annrpom annrpom requested a review from rafiss November 21, 2024 14:50
@annrpom annrpom requested a review from a team as a code owner November 21, 2024 14:50
@annrpom annrpom requested review from nameisbhaskar and vidit-bhat and removed request for a team November 21, 2024 14:50
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@annrpom annrpom requested a review from a team November 21, 2024 14:50
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

i just assigned #135801 to you as well, would you be able to add that test too if it's not too much trouble?

so the full list of tests to ignore:

TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_not_update_children_when_parent_creation_with_no_reason
TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_update_children_when_autosave_is_true_and_parent_is_new_but_child_is_not
TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_not_save_and_return_false_if_a_callback_cancelled_saving_in_either_create_or_update

CockroachDB::AdapterForeignKeyTest#test_foreign_key_violations_are_translated_to_specific_exception_with_validate_false
CockroachDB::AdapterForeignKeyTest#test_foreign_key_violations_on_insert_are_translated_to_specific_exception
CockroachDB::AdapterForeignKeyTest#test_foreign_key_violations_on_delete_are_translated_to_specific_exception

BasicsTest#test_default_values_are_deeply_dupped

`PostGISTest#test_point_to_json`: "unknown",
`TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_not_save_and_return_false_if_a_callback_cancelled_saving_in_either_create_or_update`: "unknown",
`TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_not_update_children_when_parent_creation_with_no_reason`: "unknown",
`TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_update_children_when_autosave_is_true_and_parent_is_new_but_child_is_not`: "unknown",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: these new tests are flaky, so should be in the ignoreList.

(right now they are in the blockList, which means the test will expect them to fail every 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.

ah apologies

@rafiss
Copy link
Collaborator

rafiss commented Nov 21, 2024

hm, actually the AdapterForeignKeyTest ones have only flaked twice ever, so let's hold off on ignoring those until we see that they are more unstable.

@annrpom
Copy link
Contributor Author

annrpom commented Nov 21, 2024

let's hold off on ignoring those until we see that they are more unstable

ok i'll keep my eye out for those!

@annrpom annrpom requested a review from rafiss November 21, 2024 16:42
`ActiveRecord::ConnectionAdapters::PostgreSQLAdapterTest#test_translate_no_connection_exception_to_not_established`: "pg_terminate_backend not implemented",
`TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_not_save_and_return_false_if_a_callback_cancelled_saving_in_either_create_or_update`: "flaky",
`TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_not_update_children_when_parent_creation_with_no_reason`: "flaky",
`TestAutosaveAssociationOnAHasAndBelongsToManyAssociation#test_should_update_children_when_autosave_is_true_and_parent_is_new_but_child_is_not`: "flaky",
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we add BasicsTest#test_default_values_are_deeply_dupped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my reading comprehension skills were afoot

Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

lgtm!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @nameisbhaskar and @vidit-bhat)

@rafiss rafiss added backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2 backport-24.3.x Flags PRs that need to be backported to 24.3 labels Nov 22, 2024
@annrpom
Copy link
Contributor Author

annrpom commented Nov 22, 2024

TFTR! ('-')7

ty for adding those backport labels

bors r+

@craig craig bot merged commit 7f36eb9 into cockroachdb:master Nov 23, 2024
22 of 23 checks passed
Copy link

blathers-crl bot commented Nov 23, 2024

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.


Issue #134811: branch-release-24.1, branch-release-24.3.


Issue #135801: branch-release-24.1, branch-release-24.2, branch-release-24.3.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2 backport-24.3.x Flags PRs that need to be backported to 24.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roachtest: activerecord failed roachtest: activerecord failed
3 participants