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

Fix flaky test record for skipped test #242

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Conversation

nikita8
Copy link
Contributor

@nikita8 nikita8 commented Oct 11, 2023

When test fails and are requeued it is treated as skipped for which the record_success method is executed and hence these tests are added to the flaky list which is not desired. This PR fixes it by skipping recording of flaky tests for skipped tests.

Tophat: The failing test in this build is no more in the flaky test list.

@ChrisBr
Copy link
Contributor

ChrisBr commented Oct 12, 2023

Can we add a test for this?

@nikita8 nikita8 force-pushed the nikita8/fix-flaky-tests branch from 037f006 to 6db9f3f Compare October 12, 2023 18:02
@nikita8 nikita8 requested a review from ChrisBr October 12, 2023 18:03
ruby/Gemfile Outdated
@@ -3,4 +3,4 @@ source 'https://rubygems.org'
# Specify your gem's dependencies in ci-queue.gemspec
gemspec

gem 'activesupport'
gem 'activesupport', '~> 7.0.8'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to pin this back because CI is failing on latest 7.1.1

Copy link
Contributor

Choose a reason for hiding this comment

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

We should fix it though, we already have apps on 7.1

Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed 5a7b8f0 which should fix the problem.

@casperisfine casperisfine force-pushed the nikita8/fix-flaky-tests branch from 6db9f3f to 5a7b8f0 Compare October 13, 2023 06:15
@ChrisBr ChrisBr merged commit de56717 into master Oct 13, 2023
11 checks passed
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems October 13, 2023 09:04 Inactive
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.

4 participants