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 intermittent time-based failure in delayed sidekiq spec #947

Merged
merged 2 commits into from
May 2, 2024

Conversation

mjankowski
Copy link
Contributor

This example was previously failing when it was run at exactly an "even" 10 second increment time. For example, it would fail at "12:00:10", but pass at "12:00:09" and "12:00:11". This leads to intermittent failures on CI (presumably around ~10% of runs).

Updated to use a helper method in the spec which more closely mirrors the scheduler code.

I noticed this during the CI runs from the previous PR ... and was able to verify this pattern by setting a specific time target for the Timecop.freeze line, which confirmed the pattern of fail/pass.

This example was previously failing when it was run at exactly an "even"
10 second increment time. For example, it would fail at "12:00:10", but
pass at "12:00:09" and "12:00:11". This leads to intermittent failures
on CI (presumably around ~10% of runs).

Updated to use a helper method in the spec which more closely mirrors
the scheduler code.
@konalegi
Copy link

konalegi commented May 1, 2024

It seems to be working, with no failure :wohoo: Could you please add the change log info? for instance like this https://github.com/toptal/chewy/pull/933/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR9

@konalegi
Copy link

konalegi commented May 1, 2024

I'll trigger CI 4-5 times to make sure nothing fails, and once you add changelog I'll release it :)

@mjankowski
Copy link
Contributor Author

Added changelog commit here as well.

I see there may be a DIFFERENT intermittent failure in some of these CI runs (in rake helper spec), but it looks like none so far in the sidekiq one.

@konalegi
Copy link

konalegi commented May 1, 2024

Yeah, it's different, related to sidekiq, seems to be not failing

@konalegi konalegi merged commit f45460f into toptal:master May 2, 2024
10 checks passed
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