From 6bcbe5611644b2b8eec0df6d1a6e577897142ef2 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 1 Mar 2024 10:07:35 +1000 Subject: [PATCH] DEV: Use freeze_time_safe in more places (#25949) Followup to 120a2f70a9ea3b08a39fc1fbb251f59ecf968cde, uses new method to avoid time-based spec flakiness --- spec/jobs/bulk_invite_spec.rb | 1 - .../backup_file_handler_multisite_spec.rb | 2 +- .../backup_restore/backup_file_handler_spec.rb | 2 +- .../database_restorer_multisite_spec.rb | 2 +- .../backup_restore/database_restorer_spec.rb | 2 +- .../backup_restore/meta_data_handler_spec.rb | 2 +- .../shared_context_for_backup_restore.rb | 2 +- .../system_interface_multisite_spec.rb | 2 +- .../backup_restore/system_interface_spec.rb | 2 +- .../backup_restore/uploads_restorer_spec.rb | 2 +- spec/lib/middleware/request_tracker_spec.rb | 2 +- spec/lib/post_creator_spec.rb | 5 ++--- spec/models/directory_item_spec.rb | 2 +- spec/models/incoming_links_report_spec.rb | 2 +- spec/models/post_spec.rb | 2 +- spec/models/report_spec.rb | 18 +++++++++--------- spec/models/topic_hot_scores_spec.rb | 3 +-- spec/models/topic_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- spec/rails_helper.rb | 8 ++++++++ spec/services/user_merger_spec.rb | 2 +- 21 files changed, 36 insertions(+), 31 deletions(-) diff --git a/spec/jobs/bulk_invite_spec.rb b/spec/jobs/bulk_invite_spec.rb index 3312cda7a1d..87de9748961 100644 --- a/spec/jobs/bulk_invite_spec.rb +++ b/spec/jobs/bulk_invite_spec.rb @@ -55,7 +55,6 @@ RSpec.describe Jobs::BulkInvite do it "handles daylight savings time correctly" do # EDT (-04:00) transitions to EST (-05:00) on the first Sunday in November. # Freeze time to the last Day of October, so that the creation and expiration date will be in different time zones. - Time.use_zone("Eastern Time (US & Canada)") do freeze_time DateTime.parse("2023-10-31 06:00:00 -0400") described_class.new.execute(current_user_id: east_coast_user.id, invites: invites) diff --git a/spec/lib/backup_restore/backup_file_handler_multisite_spec.rb b/spec/lib/backup_restore/backup_file_handler_multisite_spec.rb index 40a01e26bca..6b2bbbf631e 100644 --- a/spec/lib/backup_restore/backup_file_handler_multisite_spec.rb +++ b/spec/lib/backup_restore/backup_file_handler_multisite_spec.rb @@ -3,7 +3,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::BackupFileHandler, type: :multisite do - include_context "with shared stuff" + include_context "with shared backup restore context" it "works with old backup file format" do test_multisite_connection("second") do diff --git a/spec/lib/backup_restore/backup_file_handler_spec.rb b/spec/lib/backup_restore/backup_file_handler_spec.rb index c7f017692ed..5f649ad06df 100644 --- a/spec/lib/backup_restore/backup_file_handler_spec.rb +++ b/spec/lib/backup_restore/backup_file_handler_spec.rb @@ -3,7 +3,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::BackupFileHandler do - include_context "with shared stuff" + include_context "with shared backup restore context" it "works with current backup file format" do expect_decompress_and_clean_up_to_work( diff --git a/spec/lib/backup_restore/database_restorer_multisite_spec.rb b/spec/lib/backup_restore/database_restorer_multisite_spec.rb index 0da53b7513b..e63ea0377f4 100644 --- a/spec/lib/backup_restore/database_restorer_multisite_spec.rb +++ b/spec/lib/backup_restore/database_restorer_multisite_spec.rb @@ -5,7 +5,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::DatabaseRestorer, type: :multisite do subject(:restorer) { BackupRestore::DatabaseRestorer.new(logger, current_db) } - include_context "with shared stuff" + include_context "with shared backup restore context" let(:current_db) { RailsMultisite::ConnectionManagement.current_db } diff --git a/spec/lib/backup_restore/database_restorer_spec.rb b/spec/lib/backup_restore/database_restorer_spec.rb index 8d8711e9247..7c04e6cb25f 100644 --- a/spec/lib/backup_restore/database_restorer_spec.rb +++ b/spec/lib/backup_restore/database_restorer_spec.rb @@ -5,7 +5,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::DatabaseRestorer do subject(:restorer) { BackupRestore::DatabaseRestorer.new(logger, current_db) } - include_context "with shared stuff" + include_context "with shared backup restore context" let(:current_db) { RailsMultisite::ConnectionManagement.current_db } diff --git a/spec/lib/backup_restore/meta_data_handler_spec.rb b/spec/lib/backup_restore/meta_data_handler_spec.rb index 70f062e89d3..9d2151a5201 100644 --- a/spec/lib/backup_restore/meta_data_handler_spec.rb +++ b/spec/lib/backup_restore/meta_data_handler_spec.rb @@ -3,7 +3,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::MetaDataHandler do - include_context "with shared stuff" + include_context "with shared backup restore context" let!(:backup_filename) { "discourse-2019-11-18-143242-v20191108000414.tar.gz" } diff --git a/spec/lib/backup_restore/shared_context_for_backup_restore.rb b/spec/lib/backup_restore/shared_context_for_backup_restore.rb index 402a7749939..06bf51a4c6d 100644 --- a/spec/lib/backup_restore/shared_context_for_backup_restore.rb +++ b/spec/lib/backup_restore/shared_context_for_backup_restore.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_context "with shared stuff" do +RSpec.shared_context "with shared backup restore context" do let!(:logger) do Class .new do diff --git a/spec/lib/backup_restore/system_interface_multisite_spec.rb b/spec/lib/backup_restore/system_interface_multisite_spec.rb index 5b656933a40..2d0f655ffd1 100644 --- a/spec/lib/backup_restore/system_interface_multisite_spec.rb +++ b/spec/lib/backup_restore/system_interface_multisite_spec.rb @@ -5,7 +5,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::SystemInterface, type: :multisite do subject(:system_interface) { BackupRestore::SystemInterface.new(logger) } - include_context "with shared stuff" + include_context "with shared backup restore context" describe "#flush_redis" do it "removes only keys from the current site in a multisite" do diff --git a/spec/lib/backup_restore/system_interface_spec.rb b/spec/lib/backup_restore/system_interface_spec.rb index ffb4beab49c..191e220cb63 100644 --- a/spec/lib/backup_restore/system_interface_spec.rb +++ b/spec/lib/backup_restore/system_interface_spec.rb @@ -5,7 +5,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::SystemInterface do subject(:system_interface) { BackupRestore::SystemInterface.new(logger) } - include_context "with shared stuff" + include_context "with shared backup restore context" describe "readonly mode" do after { Discourse::READONLY_KEYS.each { |key| Discourse.redis.del(key) } } diff --git a/spec/lib/backup_restore/uploads_restorer_spec.rb b/spec/lib/backup_restore/uploads_restorer_spec.rb index 42e10beb791..aae63c39a54 100644 --- a/spec/lib/backup_restore/uploads_restorer_spec.rb +++ b/spec/lib/backup_restore/uploads_restorer_spec.rb @@ -6,7 +6,7 @@ require_relative "shared_context_for_backup_restore" RSpec.describe BackupRestore::UploadsRestorer do subject(:restorer) { BackupRestore::UploadsRestorer.new(logger) } - include_context "with shared stuff" + include_context "with shared backup restore context" def with_temp_uploads_directory(name: "default", with_optimized: false) Dir.mktmpdir do |directory| diff --git a/spec/lib/middleware/request_tracker_spec.rb b/spec/lib/middleware/request_tracker_spec.rb index 22e2db764b3..7757e093d7f 100644 --- a/spec/lib/middleware/request_tracker_spec.rb +++ b/spec/lib/middleware/request_tracker_spec.rb @@ -260,7 +260,7 @@ RSpec.describe Middleware::RequestTracker do # rate limiter tests depend on checks for retry-after # they can be sensitive to clock skew during test runs - freeze_time DateTime.parse("2021-01-01 01:00") + freeze_time_safe end use_redis_snapshotting diff --git a/spec/lib/post_creator_spec.rb b/spec/lib/post_creator_spec.rb index b389ee0b5ff..0f9fbec802e 100644 --- a/spec/lib/post_creator_spec.rb +++ b/spec/lib/post_creator_spec.rb @@ -680,8 +680,7 @@ RSpec.describe PostCreator do fab!(:topic) { Fabricate(:topic, user: user) } it "whispers do not mess up the public view" do - # turns out this can fail on leap years if we don't do this - freeze_time DateTime.parse("2010-01-01 12:00") + freeze_time_safe first = PostCreator.new(user, topic_id: topic.id, raw: "this is the first post").create @@ -760,7 +759,7 @@ RSpec.describe PostCreator do fab!(:topic) { Fabricate(:topic, user: user) } it "silent do not mess up the public view" do - freeze_time DateTime.parse("2010-01-01 12:00") + freeze_time_safe first = PostCreator.new(user, topic_id: topic.id, raw: "this is the first post").create diff --git a/spec/models/directory_item_spec.rb b/spec/models/directory_item_spec.rb index 05fedf0da0c..e72b088d810 100644 --- a/spec/models/directory_item_spec.rb +++ b/spec/models/directory_item_spec.rb @@ -45,7 +45,7 @@ RSpec.describe DirectoryItem do describe ".refresh!" do before do - freeze_time DateTime.parse("2017-02-02 12:00") + freeze_time_safe UserActionManager.enable end diff --git a/spec/models/incoming_links_report_spec.rb b/spec/models/incoming_links_report_spec.rb index 1bc5c498870..9712b125d29 100644 --- a/spec/models/incoming_links_report_spec.rb +++ b/spec/models/incoming_links_report_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe IncomingLinksReport do - before { freeze_time DateTime.parse("2010-01-01 6:00") } + before { freeze_time_safe } describe "integration" do it "runs correctly" do diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 1e434f06d57..91a54e778e5 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -2202,7 +2202,7 @@ RSpec.describe Post do describe "public_posts_count_per_day" do before do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe Fabricate(:post) Fabricate(:post, created_at: 1.day.ago) diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 1ed8a413261..a51c803559a 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -49,7 +49,7 @@ RSpec.describe Report do subject(:json) { Report.find("http_total_reqs").as_json } before do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe # today, an incomplete day: application_requests = [ @@ -97,7 +97,7 @@ RSpec.describe Report do describe "topics" do before do Report.clear_cache - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe user = Fabricate(:user) topics = ((0..32).to_a + [60, 61, 62, 63]).map do |i| @@ -145,7 +145,7 @@ RSpec.describe Report do let(:user) { Fabricate(:user) } it "returns a report with data" do - freeze_time DateTime.parse("2000-01-01") + freeze_time_safe user.user_visits.create(visited_at: 1.hour.from_now) user.user_visits.create(visited_at: 1.day.ago) user.user_visits.create(visited_at: 2.days.ago, mobile: true) @@ -169,7 +169,7 @@ RSpec.describe Report do let(:user) { Fabricate(:user) } it "returns a report with data" do - freeze_time DateTime.parse("2000-01-01") + freeze_time_safe user.user_visits.create(visited_at: 1.hour.from_now) user.user_visits.create(visited_at: 2.days.ago, mobile: true) user.user_visits.create(visited_at: 45.days.ago) @@ -197,7 +197,7 @@ RSpec.describe Report do context "with #{pluralized}" do before(:each) do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe if arg == :flag user = Fabricate(:user, refresh_auto_groups: true) @@ -262,7 +262,7 @@ RSpec.describe Report do context "with #{request_type}" do before do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe application_requests = [ { date: 35.days.ago.to_time, @@ -519,7 +519,7 @@ RSpec.describe Report do context "with different users/visits" do before do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe arpit = Fabricate(:user) arpit.user_visits.create(visited_at: 1.day.ago) @@ -552,7 +552,7 @@ RSpec.describe Report do context "with different activities" do before do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe UserActionManager.enable @@ -1146,7 +1146,7 @@ RSpec.describe Report do context "with data" do it "works" do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe ip = [81, 2, 69, 142] diff --git a/spec/models/topic_hot_scores_spec.rb b/spec/models/topic_hot_scores_spec.rb index 2c9bacb89e9..89e61356fa2 100644 --- a/spec/models/topic_hot_scores_spec.rb +++ b/spec/models/topic_hot_scores_spec.rb @@ -29,8 +29,7 @@ RSpec.describe TopicHotScore do end it "can correctly update like counts and post counts and account for activity" do - # freeze to specific date + time to avoid flakiness from leap years - freeze_time(DateTime.parse("2024-02-01 01:00")) + freeze_time_safe TopicHotScore.create!(topic_id: -1, score: 0.0, recent_likes: 99, recent_posters: 0) diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index e83653506ad..8684c5fcb0b 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -2519,7 +2519,7 @@ RSpec.describe Topic do describe "#listable_count_per_day" do before(:each) do - freeze_time DateTime.parse("2017-03-01 12:00") + freeze_time_safe Fabricate(:topic) Fabricate(:topic, created_at: 1.day.ago) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 354242c722c..a6b569ea041 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -442,7 +442,7 @@ RSpec.describe User do describe "#count_by_signup_date" do before(:each) do User.destroy_all - freeze_time DateTime.parse("2017-02-01 12:00") + freeze_time_safe Fabricate(:user) Fabricate(:user, created_at: 1.day.ago) Fabricate(:user, created_at: 1.day.ago) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index df77ca52296..1070c35362f 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -781,6 +781,14 @@ def set_cdn_url(cdn_url) end end +# Time.now can cause flaky tests, especially in cases like +# leap days. This method freezes time at a "safe" specific +# time (the Discourse 1.1 release date), so it will not be +# affected by further temporal disruptions. +def freeze_time_safe + freeze_time(DateTime.parse("2014-08-26 12:00:00")) +end + def freeze_time(now = Time.now) time = now datetime = now diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index ae1bcd35909..1884904b65c 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -977,7 +977,7 @@ RSpec.describe UserMerger do end it "merges user visits" do - freeze_time DateTime.parse("2010-01-01 12:00") + freeze_time_safe UserVisit.create!( user_id: source_user.id,