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

Add extra spec for time offsets / frozen time #2346

Merged
merged 1 commit into from
May 25, 2020

Conversation

JonRowe
Copy link
Member

@JonRowe JonRowe commented May 25, 2020

No description provided.

@benoittgt benoittgt self-requested a review May 25, 2020 15:24
@JonRowe JonRowe force-pushed the add-active-job-offset-example branch from 2cabe68 to 33188f6 Compare May 25, 2020 19:18
@@ -33,6 +33,8 @@ def self.find(_id)
end

RSpec.describe "ActiveJob matchers", skip: !RSpec::Rails::FeatureCheck.has_active_job? do
include ActiveSupport::Testing::TimeHelpers if defined?(ActiveSupport::Testing::TimeHelpers)
Copy link
Member

Choose a reason for hiding this comment

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

When it might not be available exactly?

Copy link
Member Author

@JonRowe JonRowe May 25, 2020

Choose a reason for hiding this comment

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

Rails 5.0 and 5.1

Copy link
Member

Choose a reason for hiding this comment

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

I mean it's clear that freeze_time isn't part of it, but TimeHelpers should be always available, and it's not necessary to check if they are defined (I hope we're requiring them somewhere).

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh ok, I was just checking freeze_time feel free to open a PR if you'd like to improve it, maybe backfill an implementation fir freeze_time if you have time?

Copy link
Member

Choose a reason for hiding this comment

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

backfill an implementation for freeze_time

Noway! :D
If just for tests - it's kind of useless since we already run those specs against Rails that have it. For usage in tests - there's TimeCop (god forgive me) - wouldn't dare to write yet another one we'll have to support.

@benoittgt
Copy link
Member

LGTM. I am looking for the sprockets bumping issue on the CI.

@JonRowe JonRowe merged commit 8310395 into master May 25, 2020
@JonRowe JonRowe deleted the add-active-job-offset-example branch May 25, 2020 20:23
JonRowe added a commit that referenced this pull request Jun 10, 2020
Add extra spec for time offsets / frozen time
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.

None yet

3 participants