From 04078317ba6577699d06cf4dccf014254dcde7a6 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Fri, 12 Feb 2021 22:15:04 +0300 Subject: [PATCH] Remove deprecated `color` and `tty` configuration and CLI options --- Changelog.md | 2 + lib/rspec/core/configuration.rb | 27 +-- lib/rspec/core/configuration_options.rb | 2 +- lib/rspec/core/drb.rb | 2 - lib/rspec/core/option_parser.rb | 8 +- spec/rspec/core/configuration_options_spec.rb | 31 +--- spec/rspec/core/configuration_spec.rb | 175 +++--------------- spec/rspec/core/drb_spec.rb | 24 +-- spec/rspec/core/runner_spec.rb | 2 +- spec/spec_helper.rb | 1 - spec/support/aruba_support.rb | 3 - 11 files changed, 42 insertions(+), 235 deletions(-) diff --git a/Changelog.md b/Changelog.md index 05a8918a46..3a611bcd97 100644 --- a/Changelog.md +++ b/Changelog.md @@ -19,6 +19,8 @@ Breaking Changes: execution result. (Phil Pirozhkov, #2862) * Skip setting the default pattern from Rake task. (Phil Pirozhkov, #2868) * Remove special `:if`/`:unless` filtering metadata. (Phil Pirozhkov, #2870) +* Remove deprecated `color` configuration option and `--color` command line + option. (Phil Pirozhkov, #2864) Enhancements: diff --git a/lib/rspec/core/configuration.rb b/lib/rspec/core/configuration.rb index d4478780bb..61727b51da 100644 --- a/lib/rspec/core/configuration.rb +++ b/lib/rspec/core/configuration.rb @@ -408,9 +408,6 @@ def bisect_runner=(value) @bisect_runner = value end - # @private - # @deprecated Use {#color_mode} = :on, instead of {#color} with {#tty} - add_setting :tty # @private attr_writer :files_to_run # @private @@ -441,7 +438,6 @@ def initialize @mock_framework = nil @files_or_directories_to_run = [] @loaded_spec_files = Set.new - @color = false @color_mode = :automatic @pattern = '**{,/*/**}/*_spec.rb' @exclude_pattern = '' @@ -803,20 +799,6 @@ def full_backtrace=(true_or_false) @backtrace_formatter.full_backtrace = true_or_false end - # Enables color output if the output is a TTY. As of RSpec 3.6, this is - # the default behavior and this option is retained only for backwards - # compatibility. - # - # @deprecated No longer recommended because of complex behavior. Instead, - # rely on the fact that TTYs will display color by default, or set - # {#color_mode} to :on to display color on a non-TTY output. - # @see color_mode - # @see color_enabled? - # @return [Boolean] - def color - value_for(:color) { @color } - end - # The mode for determining whether to display output in color. One of: # # - :automatic - the output will be in color if the output is a TTY (the @@ -839,20 +821,13 @@ def color_enabled?(output=output_stream) when :on then true when :off then false else # automatic - output_to_tty?(output) || (color && tty?) + output_to_tty?(output) end end # Set the color mode. attr_writer :color_mode - # Toggle output color. - # - # @deprecated No longer recommended because of complex behavior. Instead, - # rely on the fact that TTYs will display color by default, or set - # {:color_mode} to :on to display color on a non-TTY output. - attr_writer :color - # @private def libs=(libs) libs.map do |lib| diff --git a/lib/rspec/core/configuration_options.rb b/lib/rspec/core/configuration_options.rb index 4e74719eb6..637d654d99 100644 --- a/lib/rspec/core/configuration_options.rb +++ b/lib/rspec/core/configuration_options.rb @@ -58,7 +58,7 @@ def organize_options UNFORCED_OPTIONS = Set.new([ :requires, :profile, :drb, :libs, :files_or_directories_to_run, - :full_description, :full_backtrace, :tty + :full_description, :full_backtrace ]) UNPROCESSABLE_OPTIONS = Set.new([:formatters]) diff --git a/lib/rspec/core/drb.rb b/lib/rspec/core/drb.rb index 875c58c61a..0f92bae932 100644 --- a/lib/rspec/core/drb.rb +++ b/lib/rspec/core/drb.rb @@ -40,12 +40,10 @@ def initialize(submitted_options, filter_manager) def options argv = [] - argv << "--color" if @submitted_options[:color] argv << "--force-color" if @submitted_options[:color_mode] == :on argv << "--no-color" if @submitted_options[:color_mode] == :off argv << "--profile" if @submitted_options[:profile_examples] argv << "--backtrace" if @submitted_options[:full_backtrace] - argv << "--tty" if @submitted_options[:tty] argv << "--fail-fast" if @submitted_options[:fail_fast] argv << "--options" << @submitted_options[:custom_options_file] if @submitted_options[:custom_options_file] argv << "--order" << @submitted_options[:order] if @submitted_options[:order] diff --git a/lib/rspec/core/option_parser.rb b/lib/rspec/core/option_parser.rb index 3af69eefd7..0c77729a4b 100644 --- a/lib/rspec/core/option_parser.rb +++ b/lib/rspec/core/option_parser.rb @@ -18,7 +18,7 @@ def parse(source=nil) return { :files_or_directories_to_run => [] } if original_args.empty? args = original_args.dup - options = args.delete('--tty') ? { :tty => true } : {} + options = {} begin parser(options).parse!(args) rescue OptionParser::InvalidOption => e @@ -140,12 +140,6 @@ def parser(options) options[:full_backtrace] = true end - parser.on('-c', '--color', '--colour', '') do |_o| - # flag will be excluded from `--help` output because it is deprecated - options[:color] = true - options[:color_mode] = :automatic - end - parser.on('--force-color', '--force-colour', 'Force the output to be in color, even if the output is not a TTY') do |_o| if options[:color_mode] == :off abort "Please only use one of `--force-color` and `--no-color`" diff --git a/spec/rspec/core/configuration_options_spec.rb b/spec/rspec/core/configuration_options_spec.rb index d75a216464..05d3bb18ff 100644 --- a/spec/rspec/core/configuration_options_spec.rb +++ b/spec/rspec/core/configuration_options_spec.rb @@ -136,13 +136,6 @@ expect(config.exclusion_filter.rules).to have_key(:slow) end - it "forces color" do - opts = config_options_object(*%w[--color]) - expect(config).to receive(:force).with(:color => true) - expect(config).to receive(:force).with(:color_mode => :automatic) - opts.configure(config) - end - it "forces force_color" do opts = config_options_object(*%w[--force-color]) expect(config).to receive(:force).with(:color_mode => :on) @@ -204,36 +197,16 @@ end end - describe "-c, --color, and --colour" do - it "sets :color_mode => :automatic" do - expect(parse_options('-c')).to include(:color_mode => :automatic) - expect(parse_options('--color')).to include(:color_mode => :automatic) - expect(parse_options('--colour')).to include(:color_mode => :automatic) - end - - it "overrides previous color flag" do - expect(parse_options('--no-color', '--color')).to include(:color_mode => :automatic) - end - end - describe "--no-color" do it "sets :color_mode => :off" do expect(parse_options('--no-color')).to include(:color_mode => :off) end - - it "overrides previous color flag" do - expect(parse_options('--color', '--no-color')).to include(:color_mode => :off) - end end describe "--force-color" do it "sets :color_mode => :on" do expect(parse_options('--force-color')).to include(:color_mode => :on) end - - it "overrides previous color flag" do - expect(parse_options('--color', '--force-color')).to include(:color_mode => :on) - end end describe "-I" do @@ -365,8 +338,8 @@ end describe "files_or_directories_to_run" do - it "parses files from '-c file.rb dir/file.rb'" do - expect(parse_options("-c", "file.rb", "dir/file.rb")).to include( + it "parses files from '--no-color file.rb dir/file.rb'" do + expect(parse_options("--no-color", "file.rb", "dir/file.rb")).to include( :files_or_directories_to_run => ["file.rb", "dir/file.rb"] ) end diff --git a/spec/rspec/core/configuration_spec.rb b/spec/rspec/core/configuration_spec.rb index 75fe076a32..e0d10524b1 100644 --- a/spec/rspec/core/configuration_spec.rb +++ b/spec/rspec/core/configuration_spec.rb @@ -1265,11 +1265,20 @@ def metadata_hash(*args) end - describe "#color_mode" do - context ":automatic" do - before do - config.color_mode = :automatic - end + describe "#color_enabled?" do + it "allows overriding instance output stream with an argument" do + config.output_stream = StringIO.new + output_override = StringIO.new + + allow(config.output_stream).to receive_messages(:tty? => false) + allow(output_override).to receive_messages(:tty? => true) + + expect(config.color_enabled?).to be false + expect(config.color_enabled?(output_override)).to be true + end + + context "with color_mode :automatic" do + before { config.color_mode = :automatic } context "with output.tty?" do it "sets color_enabled?" do @@ -1288,10 +1297,8 @@ def metadata_hash(*args) end end - context ":on" do - before do - config.color_mode = :on - end + context "with color_mode :on" do + before { config.color_mode = :on } context "with output.tty?" do it "sets color_enabled?" do @@ -1310,10 +1317,8 @@ def metadata_hash(*args) end end - context ":off" do - before do - config.color_mode = :off - end + context "with color_mode :off" do + before { config.color_mode = :off } context "with output.tty?" do it "sets !color_enabled?" do @@ -1330,151 +1335,15 @@ def metadata_hash(*args) expect(config.color_enabled?).to be false end end - - it "prefers incoming cli_args" do - config.output_stream = StringIO.new - config.force :color_mode => :on - config.color_mode = :off - expect(config.color_mode).to be :on - end - end - end - - describe "#color_enabled?" do - it "allows overriding instance output stream with an argument" do - config.output_stream = StringIO.new - output_override = StringIO.new - - config.color_mode = :automatic - allow(config.output_stream).to receive_messages(:tty? => false) - allow(output_override).to receive_messages(:tty? => true) - - expect(config.color_enabled?).to be false - expect(config.color_enabled?(output_override)).to be true end end - describe "#color=" do - before { config.color_mode = :automatic } - - context "given false" do - before { config.color = false } - - context "with config.tty? and output.tty?" do - it "sets color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = true - allow(config.output_stream).to receive_messages(:tty? => true) - - expect(config.color_enabled?).to be true - expect(config.color_enabled?(output)).to be true - end - end - - context "with config.tty? and !output.tty?" do - it "does not set color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = true - allow(config.output_stream).to receive_messages(:tty? => false) - - expect(config.color_enabled?).to be false - expect(config.color_enabled?(output)).to be false - end - end - - context "with !config.tty? and output.tty?" do - it "sets color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = false - allow(config.output_stream).to receive_messages(:tty? => true) - - expect(config.color_enabled?).to be true - expect(config.color_enabled?(output)).to be true - end - end - - context "with !config.tty? and !output.tty?" do - it "does not set color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = false - allow(config.output_stream).to receive_messages(:tty? => false) - - expect(config.color_enabled?).to be false - expect(config.color_enabled?(output)).to be false - end - end - end - - context "given true" do - before { config.color = true } - - context "with config.tty? and output.tty?" do - it "sets color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = true - allow(config.output_stream).to receive_messages(:tty? => true) - - expect(config.color_enabled?).to be true - expect(config.color_enabled?(output)).to be true - end - end - - context "with config.tty? and !output.tty?" do - it "sets color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = true - allow(config.output_stream).to receive_messages(:tty? => false) - - expect(config.color_enabled?).to be true - expect(config.color_enabled?(output)).to be true - end - end - - context "with !config.tty? and output.tty?" do - it "sets color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = false - allow(config.output_stream).to receive_messages(:tty? => true) - - expect(config.color_enabled?).to be true - expect(config.color_enabled?(output)).to be true - end - end - - context "with !config.tty? and !output.tty?" do - it "does not set color_enabled?" do - output = StringIO.new - config.output_stream = output - - config.tty = false - allow(config.output_stream).to receive_messages(:tty? => false) - - expect(config.color_enabled?).to be false - expect(config.color_enabled?(output)).to be false - end - end - end - + describe '#color_mode' do it "prefers incoming cli_args" do config.output_stream = StringIO.new - allow(config.output_stream).to receive_messages(:tty? => true) - config.force :color => true - config.color = false - expect(config.color).to be true + config.force :color_mode => :on + config.color_mode = :off + expect(config.color_mode).to be :on end end diff --git a/spec/rspec/core/drb_spec.rb b/spec/rspec/core/drb_spec.rb index 8090808bc1..0d09225d28 100644 --- a/spec/rspec/core/drb_spec.rb +++ b/spec/rspec/core/drb_spec.rb @@ -125,10 +125,10 @@ def drb_filter_manager_for(args) it "preserves extra arguments" do allow(File).to receive(:exist?) { false } - expect(drb_argv_for(%w[ a --drb b --color c ])).to match_array %w[ --color a b c ] + expect(drb_argv_for(%w[ a --drb b --no-color c ])).to match_array %w[ --no-color a b c ] end - %w(--color --force-color --no-color --fail-fast --profile --backtrace --tty).each do |option| + %w(--force-color --no-color --fail-fast --profile --backtrace).each do |option| it "includes #{option}" do expect(drb_argv_for([option])).to include(option) end @@ -243,41 +243,41 @@ def drb_filter_manager_for(args) context "--drb specified in ARGV" do it "renders all the original arguments except --drb" do - argv = drb_argv_for(%w[ --drb --color --format s --example pattern + argv = drb_argv_for(%w[ --drb --no-color --format s --example pattern --profile --backtrace -I path/a -I path/b --require path/c --require path/d]) - expect(argv).to eq(%w[ --color --profile --backtrace --example pattern --format s -I path/a -I path/b --require path/c --require path/d]) + expect(argv).to eq(%w[ --no-color --profile --backtrace --example pattern --format s -I path/a -I path/b --require path/c --require path/d]) end end context "--drb specified in the options file" do it "renders all the original arguments except --drb" do - File.open("./.rspec", "w") {|f| f << "--drb --color"} - drb_argv = drb_argv_for(%w[ --tty --format s --example pattern --profile --backtrace ]) - expect(drb_argv).to eq(%w[ --color --profile --backtrace --tty --example pattern --format s]) + File.open("./.rspec", "w") {|f| f << "--drb --no-color"} + drb_argv = drb_argv_for(%w[ --format s --example pattern --profile --backtrace ]) + expect(drb_argv).to eq(%w[ --no-color --profile --backtrace --example pattern --format s]) end end context "--drb specified in ARGV and the options file" do it "renders all the original arguments except --drb" do - File.open("./.rspec", "w") {|f| f << "--drb --color"} + File.open("./.rspec", "w") {|f| f << "--drb --no-color"} argv = drb_argv_for(%w[ --drb --format s --example pattern --profile --backtrace]) - expect(argv).to eq(%w[ --color --profile --backtrace --example pattern --format s]) + expect(argv).to eq(%w[ --no-color --profile --backtrace --example pattern --format s]) end end context "--drb specified in ARGV and in as ARGV-specified --options file" do it "renders all the original arguments except --drb and --options" do - File.open("./.rspec", "w") {|f| f << "--drb --color"} + File.open("./.rspec", "w") {|f| f << "--drb --no-color"} argv = drb_argv_for(%w[ --drb --format s --example pattern --profile --backtrace]) - expect(argv).to eq(%w[ --color --profile --backtrace --example pattern --format s ]) + expect(argv).to eq(%w[ --no-color --profile --backtrace --example pattern --format s ]) end end describe "--drb, -X" do it "does not send --drb back to the parser after parsing options" do - expect(drb_argv_for(%w[--drb --color])).not_to include("--drb") + expect(drb_argv_for(%w[--drb --no-color])).not_to include("--drb") end end end diff --git a/spec/rspec/core/runner_spec.rb b/spec/rspec/core/runner_spec.rb index 8c621a5a68..7bcabc4857 100644 --- a/spec/rspec/core/runner_spec.rb +++ b/spec/rspec/core/runner_spec.rb @@ -357,7 +357,7 @@ def interrupt end it "assigns submitted ConfigurationOptions to @options" do - config_options = ConfigurationOptions.new(%w[--color]) + config_options = ConfigurationOptions.new(%w[--no-color]) runner = Runner.new(config_options) expect(runner.instance_exec { @options }).to be(config_options) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6063f2f1db..fffaa85161 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -76,7 +76,6 @@ def handle_current_dir_change # runtime options c.raise_errors_for_deprecations! - c.color = true c.include CommonHelpers c.expect_with :rspec do |expectations| diff --git a/spec/support/aruba_support.rb b/spec/support/aruba_support.rb index 5f8383f2db..9ef959d669 100644 --- a/spec/support/aruba_support.rb +++ b/spec/support/aruba_support.rb @@ -35,8 +35,6 @@ module ArubaLoader attr_reader :last_cmd_stdout, :last_cmd_stderr, :last_cmd_exit_status def run_command(cmd) - RSpec.configuration.color = true - temp_stdout = StringIO.new temp_stderr = StringIO.new @@ -51,7 +49,6 @@ def run_command(cmd) end ensure RSpec.reset - RSpec.configuration.color = true # Ensure it gets cached with a proper value -- if we leave it set to nil, # and the next spec operates in a different dir, it could get set to an