Enable verbose mode in test and report warnings as errors
We recently let a few very easy to avoid warnings get merged. The root cause is that locally the test suite doesn't run in verbose mode unless you explictly pass `-w`. On CI warnings are enabled, but there is no reason to look at the build output unless something is failing. And even if one wanted to do that, that would be particularly work intensive since warnings may be specific to a Ruby version etc. Because of this I believe we should: - Always run the test suite with warnings enabled. - Raise an error if a warning is unexpected. We've been using this pattern for a long time at Shopify both in private and public repositories.
This commit is contained in:
parent
ca11431647
commit
d917896f45
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "action_cable"
|
||||
require "active_support/testing/autorun"
|
||||
require "active_support/testing/method_call_assertions"
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_support/core_ext/kernel/reporting"
|
||||
|
||||
# These are the normal settings that will be set up by Railties
|
||||
|
@ -1,5 +1,7 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
|
||||
$:.unshift File.expand_path("lib", __dir__)
|
||||
|
||||
require "active_support/core_ext/kernel/reporting"
|
||||
|
@ -7,7 +7,8 @@ class Builder
|
||||
|
||||
def call(template, source)
|
||||
require_engine
|
||||
"xml = ::Builder::XmlMarkup.new(indent: 2, target: output_buffer.raw);" \
|
||||
# the double assignment is to silence "assigned but unused variable" warnings
|
||||
"xml = xml = ::Builder::XmlMarkup.new(indent: 2, target: output_buffer.raw);" \
|
||||
"#{source};" \
|
||||
"output_buffer.to_s"
|
||||
end
|
||||
|
@ -1,5 +1,7 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
|
||||
$:.unshift File.expand_path("lib", __dir__)
|
||||
|
||||
ENV["TMPDIR"] = File.expand_path("tmp", __dir__)
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_job"
|
||||
require "support/job_buffer"
|
||||
|
||||
|
@ -6,6 +6,7 @@ module SerializeCastValue # :nodoc:
|
||||
def self.included(klass)
|
||||
unless klass.respond_to?(:included_serialize_cast_value)
|
||||
klass.singleton_class.attr_accessor :included_serialize_cast_value
|
||||
klass.silence_redefinition_of_method(:itself_if_class_included_serialize_cast_value)
|
||||
klass.attr_reader :itself_if_class_included_serialize_cast_value
|
||||
end
|
||||
klass.included_serialize_cast_value = true
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_model"
|
||||
|
||||
# Show backtraces for deprecated behavior for quicker cleanup.
|
||||
|
@ -311,6 +311,7 @@ def initialize(...)
|
||||
|
||||
@max_identifier_length = nil
|
||||
@type_map = nil
|
||||
@raw_connection = nil
|
||||
|
||||
@use_insert_returning = @config.key?(:insert_returning) ? self.class.type_cast_config_to_boolean(@config[:insert_returning]) : true
|
||||
end
|
||||
|
@ -8,6 +8,10 @@ module ActiveRecord
|
||||
module ConnectionAdapters
|
||||
class PostgreSQLAdapter < AbstractAdapter
|
||||
class InactivePgConnection
|
||||
def initialize
|
||||
@raw_connection = nil
|
||||
end
|
||||
|
||||
def query(*args)
|
||||
raise PG::Error
|
||||
end
|
||||
|
@ -413,7 +413,7 @@ class EnumTest < ActiveRecord::TestCase
|
||||
e = assert_raises(ArgumentError) do
|
||||
Class.new(ActiveRecord::Base) do
|
||||
self.table_name = "books"
|
||||
enum :status, {}
|
||||
enum(:status, {}, **{})
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_support"
|
||||
require "active_support/testing/autorun"
|
||||
require "active_support/testing/method_call_assertions"
|
||||
|
@ -1,5 +1,7 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
|
||||
ENV["RAILS_ENV"] ||= "test"
|
||||
require_relative "dummy/config/environment.rb"
|
||||
|
||||
|
27
activesupport/lib/active_support/testing/strict_warnings.rb
Normal file
27
activesupport/lib/active_support/testing/strict_warnings.rb
Normal file
@ -0,0 +1,27 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
$VERBOSE = true
|
||||
Warning[:deprecated] = true
|
||||
|
||||
module RaiseWarnings
|
||||
PROJECT_ROOT = File.expand_path("../../../../", __dir__)
|
||||
ALLOWED_WARNINGS = Regexp.union(
|
||||
/circular require considered harmful.*delayed_job/, # Bug in delayed job.
|
||||
|
||||
# Expected non-verbose warning emitted by Rails.
|
||||
/Ignoring .*\.yml because it has expired/,
|
||||
/Failed to validate the schema cache because/,
|
||||
)
|
||||
def warn(message, *)
|
||||
super
|
||||
|
||||
return unless message.include?(PROJECT_ROOT)
|
||||
return if ALLOWED_WARNINGS.match?(message)
|
||||
return unless ENV["RAILS_STRICT_WARNINGS"] || ENV["CI"]
|
||||
|
||||
raise message
|
||||
end
|
||||
ruby2_keywords :warn if respond_to?(:ruby2_keywords, true)
|
||||
end
|
||||
|
||||
Warning.singleton_class.prepend(RaiseWarnings)
|
@ -1,5 +1,7 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
|
||||
ORIG_ARGV = ARGV.dup
|
||||
|
||||
require "bundler/setup"
|
||||
|
@ -779,16 +779,19 @@ def assert_callbacks_called_with(matchers = {})
|
||||
lambda do |*args|
|
||||
message, callstack, deprecator = args
|
||||
bindings << binding
|
||||
[message, callstack, deprecator]
|
||||
end,
|
||||
|
||||
lambda do |message, *other|
|
||||
callstack, deprecator = other
|
||||
bindings << binding
|
||||
[callstack, deprecator]
|
||||
end,
|
||||
|
||||
lambda do |message, callstack, *details|
|
||||
deprecation_horizon, gem_name = details
|
||||
bindings << binding
|
||||
[deprecation_horizon, gem_name]
|
||||
end,
|
||||
]
|
||||
|
||||
|
@ -181,6 +181,7 @@ def namespaced_commands
|
||||
attr_reader :current_subcommand
|
||||
|
||||
def invoke_command(command, *) # :nodoc:
|
||||
@current_subcommand ||= nil
|
||||
original_subcommand, @current_subcommand = @current_subcommand, command.name
|
||||
super
|
||||
ensure
|
||||
|
@ -1,5 +1,7 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
|
||||
ENV["RAILS_ENV"] ||= "test"
|
||||
|
||||
require "stringio"
|
||||
|
@ -247,7 +247,9 @@ def show
|
||||
config.eager_load = true
|
||||
RUBY
|
||||
|
||||
silence_warnings do
|
||||
require "#{app_path}/config/environment"
|
||||
end
|
||||
assert_not ActiveRecord::Base.connection.schema_cache.data_sources("posts")
|
||||
end
|
||||
|
||||
|
@ -78,7 +78,7 @@ class RESTfulController < ApplicationController
|
||||
end
|
||||
RUBY
|
||||
|
||||
app_file "config/initializers/autoload_Y.rb", "Y"
|
||||
app_file "config/initializers/autoload_Y.rb", "Y.succ"
|
||||
|
||||
# Preconditions.
|
||||
assert_not Object.const_defined?(:X)
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_support"
|
||||
require "active_support/testing/autorun"
|
||||
require "rails/configuration"
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_support/test_case"
|
||||
require "active_support/testing/autorun"
|
||||
require "rails/generators/rails/app/app_generator"
|
||||
|
@ -1,5 +1,6 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "active_support/test_case"
|
||||
require "active_support/testing/autorun"
|
||||
require "rails/generators/app_base"
|
||||
|
@ -8,6 +8,7 @@
|
||||
#
|
||||
# It is also good to know what is the bare minimum to get
|
||||
# Rails booted up.
|
||||
require "active_support/testing/strict_warnings"
|
||||
require "fileutils"
|
||||
require "shellwords"
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user