Merge pull request #50188 from skipkayhil/hm-fix-ci-env

Fix test infra depending on CI=true

actionmailbox/test/test_helper.rb

@@ -5,8 +5,6 @@ require "active_support/testing/strict_warnings"
 ENV["RAILS_ENV"] = "test"
 ENV["RAILS_INBOUND_EMAIL_PASSWORD"] = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL"
 
-require_relative "../../tools/test_common"
-
 require_relative "../test/dummy/config/environment"
 ActiveRecord::Migrator.migrations_paths = [File.expand_path("../test/dummy/db/migrate", __dir__)]
 require "rails/test_help"
@@ -55,3 +53,5 @@ class BounceMailer < ActionMailer::Base
     end
   end
 end
+
+require_relative "../../tools/test_common"

actionpack/test/dispatch/system_testing/screenshot_helper_test.rb

@@ -152,17 +152,18 @@ class ScreenshotHelperTest < ActiveSupport::TestCase
     assert_match %r|url=artifact://.+?tmp/screenshots/1_x\.png|, display_image_actual
   end
 
-  test "take_failed_screenshot persists the image path in the test metadata" do
-    skip "Older versions of Minitest don't support test metadata." unless Minitest::Runnable.method_defined?(:metadata)
+  # minitest doesn't support metadata in the version locked on Ruby 2.7
+  if RUBY_VERSION > "3.0"
+    test "take_failed_screenshot persists the image path in the test metadata" do
+      Rails.stub :root, Pathname.getwd do
+        @new_test.stub :passed?, false do
+          Capybara::Session.stub :instance_created?, true do
+            @new_test.stub :save_image, nil do
+              @new_test.stub :show, -> (_) { } do
+                @new_test.take_failed_screenshot
 
-    Rails.stub :root, Pathname.getwd do
-      @new_test.stub :passed?, false do
-        Capybara::Session.stub :instance_created?, true do
-          @new_test.stub :save_image, nil do
-            @new_test.stub :show, -> (_) { } do
-              @new_test.take_failed_screenshot
-
-              assert_equal @new_test.send(:relative_image_path), @new_test.metadata[:failure_screenshot_path]
+                assert_equal @new_test.send(:relative_image_path), @new_test.metadata[:failure_screenshot_path]
+              end
             end
           end
         end

activejob/test/cases/queuing_test.rb

@@ -26,7 +26,6 @@ class QueuingTest < ActiveSupport::TestCase
     result = HelloJob.set(wait_until: 1.second.ago).perform_later "Jamie"
     assert result
   rescue NotImplementedError
-    skip
   end
 
   test "job returned by enqueue has the arguments available" do
@@ -38,7 +37,6 @@ class QueuingTest < ActiveSupport::TestCase
     job = HelloJob.set(wait_until: Time.utc(2014, 1, 1)).perform_later
     assert_equal Time.utc(2014, 1, 1), job.scheduled_at
   rescue NotImplementedError
-    skip
   end
 
   test "job is yielded to block after enqueue with successfully_enqueued property set" do
@@ -82,7 +80,6 @@ class QueuingTest < ActiveSupport::TestCase
     ActiveJob.perform_all_later(scheduled_job_1, scheduled_job_2)
     assert_equal ["Scheduled 2014 says hello", "Scheduled 2015 says hello"], JobBuffer.values.sort
   rescue NotImplementedError
-    skip
   end
 
   test "perform_all_later instrumentation" do

activejob/test/helper.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require_relative "../../tools/test_common"
-
 require "active_support/testing/strict_warnings"
 require "active_job"
 require "support/job_buffer"
@@ -23,3 +21,5 @@ require "active_support/testing/autorun"
 def adapter_is?(*adapter_class_symbols)
   adapter_class_symbols.map(&:to_s).include? ActiveJob::Base.queue_adapter_name
 end
+
+require_relative "../../tools/test_common"

activejob/test/integration/queuing_test.rb

@@ -13,58 +13,60 @@ class QueuingTest < ActiveSupport::TestCase
     assert_job_executed
   end
 
-  test "should not run jobs queued on a non-listening queue" do
-    skip if adapter_is?(:inline, :async, :sucker_punch)
-    old_queue = TestJob.queue_name
+  unless adapter_is?(:inline, :async, :sucker_punch)
+    test "should not run jobs queued on a non-listening queue" do
+      old_queue = TestJob.queue_name
 
-    begin
-      TestJob.queue_as :some_other_queue
-      TestJob.perform_later @id
-      wait_for_jobs_to_finish_for(2.seconds)
-      assert_job_not_executed
-    ensure
-      TestJob.queue_name = old_queue
+      begin
+        TestJob.queue_as :some_other_queue
+        TestJob.perform_later @id
+        wait_for_jobs_to_finish_for(2.seconds)
+        assert_job_not_executed
+      ensure
+        TestJob.queue_name = old_queue
+      end
     end
   end
 
-  test "should supply a wrapped class name to Sidekiq" do
-    skip unless adapter_is?(:sidekiq)
-    Sidekiq::Testing.fake! do
+  if adapter_is?(:sidekiq)
+    test "should supply a wrapped class name to Sidekiq" do
+      Sidekiq::Testing.fake! do
+        ::HelloJob.perform_later
+        hash = ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper.jobs.first
+        assert_equal "ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper", hash["class"]
+        assert_equal "HelloJob", hash["wrapped"]
+      end
+    end
+
+    test "should access provider_job_id inside Sidekiq job" do
+      Sidekiq::Testing.inline! do
+        job = ::ProviderJidJob.perform_later
+        assert_equal "Provider Job ID: #{job.provider_job_id}", JobBuffer.last_value
+      end
+    end
+  end
+
+  if adapter_is?(:delayed_job)
+    test "should supply a wrapped class name to DelayedJob" do
       ::HelloJob.perform_later
-      hash = ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper.jobs.first
-      assert_equal "ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper", hash["class"]
-      assert_equal "HelloJob", hash["wrapped"]
+      job = Delayed::Job.first
+      assert_match(/HelloJob \[[0-9a-f-]+\] from DelayedJob\(default\) with arguments: \[\]/, job.name)
     end
   end
 
-  test "should access provider_job_id inside Sidekiq job" do
-    skip unless adapter_is?(:sidekiq)
-    Sidekiq::Testing.inline! do
-      job = ::ProviderJidJob.perform_later
-      assert_equal "Provider Job ID: #{job.provider_job_id}", JobBuffer.last_value
+  if adapter_is?(:resque)
+    test "resque JobWrapper should have instance variable queue" do
+      job = ::HelloJob.set(wait: 5.seconds).perform_later
+      hash = Resque.decode(Resque.find_delayed_selection { true }[0])
+      assert_equal hash["queue"], job.queue_name
     end
   end
 
-  test "should supply a wrapped class name to DelayedJob" do
-    skip unless adapter_is?(:delayed_job)
-    ::HelloJob.perform_later
-    job = Delayed::Job.first
-    assert_match(/HelloJob \[[0-9a-f-]+\] from DelayedJob\(default\) with arguments: \[\]/, job.name)
-  end
-
-  test "resque JobWrapper should have instance variable queue" do
-    skip unless adapter_is?(:resque)
-    job = ::HelloJob.set(wait: 5.seconds).perform_later
-    hash = Resque.decode(Resque.find_delayed_selection { true }[0])
-    assert_equal hash["queue"], job.queue_name
-  end
-
   test "should not run job enqueued in the future" do
     TestJob.set(wait: 10.minutes).perform_later @id
     wait_for_jobs_to_finish_for(5.seconds)
     assert_job_not_executed
   rescue NotImplementedError
-    skip
   end
 
   test "should run job enqueued in the future at the specified time" do
@@ -74,7 +76,6 @@ class QueuingTest < ActiveSupport::TestCase
     wait_for_jobs_to_finish_for(10.seconds)
     assert_job_executed
   rescue NotImplementedError
-    skip
   end
 
   test "should run job bulk enqueued in the future at the specified time" do
@@ -84,25 +85,22 @@ class QueuingTest < ActiveSupport::TestCase
     wait_for_jobs_to_finish_for(10.seconds)
     assert_job_executed
   rescue NotImplementedError
-    skip
   end
 
-  test "should supply a provider_job_id when available for immediate jobs" do
-    skip unless adapter_is?(:async, :delayed_job, :sidekiq, :queue_classic)
-    test_job = TestJob.perform_later @id
-    assert test_job.provider_job_id, "Provider job id should be set by provider"
+  if adapter_is?(:async, :delayed_job, :sidekiq, :queue_classic)
+    test "should supply a provider_job_id when available for immediate jobs" do
+      test_job = TestJob.perform_later @id
+      assert test_job.provider_job_id, "Provider job id should be set by provider"
+    end
+
+    test "should supply a provider_job_id when available for delayed jobs" do
+      delayed_test_job = TestJob.set(wait: 1.minute).perform_later @id
+      assert delayed_test_job.provider_job_id, "Provider job id should by set for delayed jobs by provider"
+    end
   end
 
-  test "should supply a provider_job_id when available for delayed jobs" do
-    skip unless adapter_is?(:async, :delayed_job, :sidekiq, :queue_classic)
-    delayed_test_job = TestJob.set(wait: 1.minute).perform_later @id
-    assert delayed_test_job.provider_job_id, "Provider job id should by set for delayed jobs by provider"
-  end
-
-  test "current locale is kept while running perform_later" do
-    skip if adapter_is?(:inline)
-
-    begin
+  unless adapter_is?(:inline)
+    test "current locale is kept while running perform_later" do
       I18n.available_locales = [:en, :de]
       I18n.locale = :de
 
@@ -114,12 +112,8 @@ class QueuingTest < ActiveSupport::TestCase
       I18n.available_locales = [:en]
       I18n.locale = :en
     end
-  end
 
-  test "current timezone is kept while running perform_later" do
-    skip if adapter_is?(:inline)
-
-    begin
+    test "current timezone is kept while running perform_later" do
       current_zone = Time.zone
       Time.zone = "Hawaii"
 
@@ -132,28 +126,28 @@ class QueuingTest < ActiveSupport::TestCase
     end
   end
 
-  test "should run job with higher priority first" do
-    skip unless adapter_is?(:delayed_job)
-
-    wait_until = Time.now + 3.seconds
-    TestJob.set(wait_until: wait_until, priority: 20).perform_later "#{@id}.1"
-    TestJob.set(wait_until: wait_until, priority: 10).perform_later "#{@id}.2"
-    wait_for_jobs_to_finish_for(10.seconds)
-    assert_job_executed "#{@id}.1"
-    assert_job_executed "#{@id}.2"
-    assert_job_executed_before("#{@id}.2", "#{@id}.1")
+  if adapter_is?(:delayed_job)
+    test "should run job with higher priority first" do
+      wait_until = Time.now + 3.seconds
+      TestJob.set(wait_until: wait_until, priority: 20).perform_later "#{@id}.1"
+      TestJob.set(wait_until: wait_until, priority: 10).perform_later "#{@id}.2"
+      wait_for_jobs_to_finish_for(10.seconds)
+      assert_job_executed "#{@id}.1"
+      assert_job_executed "#{@id}.2"
+      assert_job_executed_before("#{@id}.2", "#{@id}.1")
+    end
   end
 
-  test "should run job with higher priority first in Backburner" do
-    skip unless adapter_is?(:backburner)
-
-    jobs_manager.tube.pause(3)
-    TestJob.set(priority: 20).perform_later "#{@id}.1"
-    TestJob.set(priority: 10).perform_later "#{@id}.2"
-    wait_for_jobs_to_finish_for(10.seconds)
-    assert_job_executed "#{@id}.1"
-    assert_job_executed "#{@id}.2"
-    assert_job_executed_before("#{@id}.2", "#{@id}.1")
+  if adapter_is?(:backburner)
+    test "should run job with higher priority first in Backburner" do
+      jobs_manager.tube.pause(3)
+      TestJob.set(priority: 20).perform_later "#{@id}.1"
+      TestJob.set(priority: 10).perform_later "#{@id}.2"
+      wait_for_jobs_to_finish_for(10.seconds)
+      assert_job_executed "#{@id}.1"
+      assert_job_executed "#{@id}.2"
+      assert_job_executed_before("#{@id}.2", "#{@id}.1")
+    end
   end
 
   private

activesupport/lib/active_support/testing/strict_warnings.rb

@@ -28,7 +28,7 @@ module ActiveSupport
 
       return unless message.include?(PROJECT_ROOT)
       return if ALLOWED_WARNINGS.match?(message)
-      return unless ENV["RAILS_STRICT_WARNINGS"] || ENV["CI"]
+      return unless ENV["RAILS_STRICT_WARNINGS"] || ENV["BUILDKITE"]
 
       raise message
     end

activesupport/test/cache/behaviors/cache_logging_behavior.rb

@@ -77,7 +77,7 @@ module CacheLoggingBehavior
       assert_match pattern, io.string
     end
 
-    def key_pattern(key, namespace: @namespace)
+    def key_pattern(key, namespace: defined?(@namespace) && @namespace)
       /#{Regexp.escape namespace.to_s}#{":" if namespace}#{Regexp.escape key}/
     end
 end

activesupport/test/cache/behaviors/cache_store_format_version_behavior.rb

@@ -47,9 +47,9 @@ module CacheStoreFormatVersionBehavior
           lookup_store.send(:serialize_entry, ActiveSupport::Cache::Entry.new(["value"] * 100))
         end
 
-        skip if !serialized.is_a?(String)
-
-        assert_operator serialized, :start_with?, uncompressed_signature
+        if serialized.is_a?(String)
+          assert_operator serialized, :start_with?, uncompressed_signature
+        end
       end
 
       test "format version #{format_version.inspect} uses correct signature for compressed entries" do

activesupport/test/executor_test.rb

@@ -225,31 +225,33 @@ class ExecutorTest < ActiveSupport::TestCase
     assert_equal [:state_a, :state_b, :state_d, :state_c], supplied_state
   end
 
-  def test_class_serial_is_unaffected
-    skip if !defined?(RubyVM) || !RubyVM.stat.has_key?(:class_serial)
+  if RUBY_VERSION < "3.2"
+    def test_class_serial_is_unaffected
+      skip if !defined?(RubyVM) || !RubyVM.stat.has_key?(:class_serial)
 
-    hook = Class.new do
-      define_method(:run) do
-        nil
-      end
+      hook = Class.new do
+        define_method(:run) do
+          nil
+        end
 
-      define_method(:complete) do |state|
-        nil
-      end
-    end.new
+        define_method(:complete) do |state|
+          nil
+        end
+      end.new
 
-    executor.register_hook(hook)
+      executor.register_hook(hook)
 
-    # Warm-up to trigger any pending autoloads
-    executor.wrap { }
+      # Warm-up to trigger any pending autoloads
+      executor.wrap { }
 
-    before = RubyVM.stat(:class_serial)
-    executor.wrap { }
-    executor.wrap { }
-    executor.wrap { }
-    after = RubyVM.stat(:class_serial)
+      before = RubyVM.stat(:class_serial)
+      executor.wrap { }
+      executor.wrap { }
+      executor.wrap { }
+      after = RubyVM.stat(:class_serial)
 
-    assert_equal before, after
+      assert_equal before, after
+    end
   end
 
   def test_separate_classes_can_wrap

railties/test/application/rake/dbs_test.rb

@@ -232,8 +232,6 @@ module ApplicationTests
 
       def with_bad_permissions
         Dir.chdir(app_path) do
-          skip "Can't avoid permissions as root" if Process.uid.zero?
-
           set_database_url
           FileUtils.chmod("-w", "db")
           yield
@@ -241,14 +239,25 @@ module ApplicationTests
         end
       end
 
-      test "db:create failure because bad permissions" do
-        with_bad_permissions do
-          output = rails("db:create", allow_failure: true)
-          assert_match("Couldn't create '#{database_url_db_name}' database. Please check your configuration.", output)
-          assert_equal 1, $?.exitstatus
+      unless Process.uid.zero?
+        test "db:create failure because bad permissions" do
+          with_bad_permissions do
+            output = rails("db:create", allow_failure: true)
+            assert_match("Couldn't create '#{database_url_db_name}' database. Please check your configuration.", output)
+            assert_equal 1, $?.exitstatus
+          end
+        end
+
+        test "db:drop failure because bad permissions" do
+          with_database_existing do
+            with_bad_permissions do
+              output = rails("db:drop", allow_failure: true)
+              assert_match(/Couldn't drop/, output)
+              assert_equal 1, $?.exitstatus
+            end
+          end
         end
       end
-
       test "db:create works when schema cache exists and database does not exist" do
         use_postgresql
 
@@ -268,16 +277,6 @@ module ApplicationTests
         assert_match(/does not exist/, output)
       end
 
-      test "db:drop failure because bad permissions" do
-        with_database_existing do
-          with_bad_permissions do
-            output = rails("db:drop", allow_failure: true)
-            assert_match(/Couldn't drop/, output)
-            assert_equal 1, $?.exitstatus
-          end
-        end
-      end
-
       test "db:truncate_all truncates all non-internal tables" do
         Dir.chdir(app_path) do
           rails "generate", "model", "book", "title:string"

tools/test_common.rb

@@ -5,9 +5,7 @@ if ENV["BUILDKITE"]
   ENV.delete("CI") # CI has affect on the applications, and we don't want it applied to the apps.
 
   Minitest::Ci.report_dir = File.join(__dir__, "../test-reports/#{ENV['BUILDKITE_JOB_ID']}")
-end
 
-if ENV["CI"]
   module DisableSkipping # :nodoc:
     private
       def skip(message = nil, *)