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 tests on ruby 3.1, 3.2, rails 7.0, drop support for rails 5.1 #22

Merged
merged 5 commits into from Jun 27, 2023

Conversation

razumau
Copy link
Contributor

@razumau razumau commented May 17, 2023

Adds tests on Ruby 3.1 and 3.2 and Rails 7.0. Drops support for Rails 5.1.

Since Rails 7, datetime columns have precision 6 by default. I set the same value explicitly in the test database and switched time comparisons in several tests to rounding timestamps with .to_i.

@razumau razumau force-pushed the jury.razumau/ruby_3_rails_7 branch 5 times, most recently from f825803 to e4f9c09 Compare May 17, 2023 13:37
@razumau razumau requested a review from a team May 17, 2023 13:50
@razumau razumau marked this pull request as ready for review May 17, 2023 13:50
.ruby-version Outdated Show resolved Hide resolved
@@ -57,7 +57,7 @@ class QueueAuditTest < Minitest::Test
Timecop.freeze(Time.now) do
@audit.complete!

assert_equal Time.now, @audit.completed_at
assert_equal Time.now.to_i, @audit.completed_at.to_i
Copy link
Member

Choose a reason for hiding this comment

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

Why do we set the precision to 6 digits in the schema and then use 0 digits when we compare timestamps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were separate changes: I set precision to 6 for consistency across tests, and then decided that to_i’s precision is good enough for these tests. Do you think we should test these more precisely?

Copy link
Member

Choose a reason for hiding this comment

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

The to_i conversion should not be necessary, especially inside a Timecop.freeze block. If the to_i is necessary for making some of the tests pass, I suspect those tests fail once in a while when rounding 3.999997 to 3 and 4.000002 to 4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworked it to use Time#floor. Maybe there’s a better way to do this? The problem is that while in the database we now only have precision 6, Time.now still has additional nanoseconds, so we have to either cut them off or make a comparison less strict.

@razumau razumau force-pushed the jury.razumau/ruby_3_rails_7 branch 2 times, most recently from 15a6995 to 77e51b8 Compare June 14, 2023 12:55
@razumau razumau force-pushed the jury.razumau/ruby_3_rails_7 branch 2 times, most recently from 0c8b807 to cdab23a Compare June 26, 2023 14:49
@razumau razumau force-pushed the jury.razumau/ruby_3_rails_7 branch from cdab23a to 4e41edb Compare June 26, 2023 14:49
@razumau razumau requested a review from bquorning June 26, 2023 14:52
@razumau razumau merged commit 7c01114 into master Jun 27, 2023
15 checks passed
@razumau razumau deleted the jury.razumau/ruby_3_rails_7 branch June 27, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants