Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Support bisect in spec opts #2271

Merged
merged 5 commits into from
Jun 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ rvm:
- 1.9.3
- 2.0.0
- 2.1
- 2.2
- 2.2.5
- 2.3.1
- ruby-head
- ree
Expand Down
1 change: 1 addition & 0 deletions lib/rspec/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
option_parser
configuration_options
runner
invocations
example
shared_example_group
example_group
Expand Down
24 changes: 23 additions & 1 deletion lib/rspec/core/bisect/runner.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
RSpec::Support.require_rspec_core "shell_escape"
require 'open3'
require 'shellwords'

module RSpec
module Core
Expand Down Expand Up @@ -71,24 +72,45 @@ def run_locations(*capture_args)
# https://github.com/jruby/jruby/issues/2766
if Open3.respond_to?(:capture2e) && !RSpec::Support::Ruby.jruby?
def run_command(cmd)
Open3.capture2e(cmd).first
Open3.capture2e(bisect_environment_hash, cmd).first
end
else # for 1.8.7
# :nocov:
def run_command(cmd)
out = err = nil

original_spec_opts = ENV['SPEC_OPTS']
ENV['SPEC_OPTS'] = spec_opts_without_bisect

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 did you try using the open3 feature of passing in the env opts?

The parameters env, cmd, and opts are passed to Process.spawn

env: hash
name => val : set the environment variable
name => nil : unset the environment variable
options: hash
clearing environment variables:
:unsetenv_others => true : clear environment variables except specified by env
:unsetenv_others => false : don't clear (default)
If a hash is given as env, the environment is updated by env before exec(2) in the child process. If a pair in env has nil as the value, the variable is deleted.

set FOO as BAR and unset BAZ.
pid = spawn({"FOO"=>"BAR", "BAZ"=>nil}, command)

The :unsetenv_others key in options specifies to clear environment variables, other than specified by env.

pid = spawn(command, :unsetenv_others=>true) # no environment variable
pid = spawn({"FOO"=>"BAR"}, command, :unsetenv_others=>true) # FOO only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The upside of supporting ruby 1.8.7 is that I get to use the old hash syntax without feeling guilt. The downside is that some of stdlib wasn't as capable back then, and Open3/Process.spawn is one of those bits, unfortunately. I've used the new passed environment where possible, but for 1.8.7 it's either put the new env in the command string or mutate ENV, so I plumped for the latter.

Open3.popen3(cmd) do |_, stdout, stderr|
# Reading the streams blocks until the process is complete
out = stdout.read
err = stderr.read
end

"Stdout:\n#{out}\n\nStderr:\n#{err}"
ensure
ENV['SPEC_OPTS'] = original_spec_opts
end
# :nocov:
end

def bisect_environment_hash
if ENV.key?('SPEC_OPTS')
{ 'SPEC_OPTS' => spec_opts_without_bisect }
else
{}
end
end

def spec_opts_without_bisect
Shellwords.join(
Shellwords.split(ENV.fetch('SPEC_OPTS', '')).reject do |arg|
arg =~ /^--bisect/
end
)
end

def reusable_cli_options
@reusable_cli_options ||= begin
opts = original_cli_args_without_locations
Expand Down
3 changes: 3 additions & 0 deletions lib/rspec/core/configuration_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ def configure_filter_manager(filter_manager)
# @return [Hash] the final merged options, drawn from all external sources
attr_reader :options

# @return [Array<String>] the original command-line arguments
attr_reader :args

private

def organize_options
Expand Down
67 changes: 67 additions & 0 deletions lib/rspec/core/invocations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
module RSpec
module Core
# @private
module Invocations
# @private
class InitializeProject
def call(*_args)
RSpec::Support.require_rspec_core "project_initializer"
ProjectInitializer.new.run
0
end
end

# @private
class DRbWithFallback
def call(options, err, out)
require 'rspec/core/drb'
begin
return DRbRunner.new(options).run(err, out)
rescue DRb::DRbConnError
err.puts "No DRb server is running. Running in local process instead ..."
end
RSpec::Core::Runner.new(options).run(err, out)
end
end

# @private
class Bisect
def call(options, _err, _out)
RSpec::Support.require_rspec_core "bisect/coordinator"

success = RSpec::Core::Bisect::Coordinator.bisect_with(
options.args,
RSpec.configuration,
bisect_formatter_for(options.options[:bisect])
)

success ? 0 : 1
end

private

def bisect_formatter_for(argument)
return Formatters::BisectDebugFormatter if argument == "verbose"
Formatters::BisectProgressFormatter
end
end

# @private
class PrintVersion
def call(_options, _err, out)
out.puts RSpec::Core::Version::STRING
0
end
end

# @private
PrintHelp = Struct.new(:parser, :invalid_options) do
def call(_options, _err, out)
# Removing the blank invalid options from the output.
out.puts parser.to_s.gsub(/^\s+(#{invalid_options.join('|')})\s*$\n/, '')
0
end
end
end
end
end
50 changes: 10 additions & 40 deletions lib/rspec/core/option_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def parse(source=nil)
# rubocop:disable MethodLength
# rubocop:disable Metrics/AbcSize
# rubocop:disable CyclomaticComplexity
# rubocop:disable PerceivedComplexity
def parser(options)
OptionParser.new do |parser|
parser.banner = "Usage: rspec [options] [files or directories]\n\n"
Expand Down Expand Up @@ -68,7 +69,8 @@ def parser(options)

parser.on('--bisect[=verbose]', 'Repeatedly runs the suite in order to isolate the failures to the ',
' smallest reproducible case.') do |argument|
bisect_and_exit(argument)
options[:bisect] = argument || true
options[:runner] = RSpec::Core::Invocations::Bisect.new
end

parser.on('--[no-]fail-fast[=COUNT]', 'Abort the run after a certain number of failures (1 by default).') do |argument|
Expand Down Expand Up @@ -96,16 +98,17 @@ def parser(options)
options[:dry_run] = true
end

parser.on('-X', '--[no-]drb', 'Run examples via DRb.') do |o|
options[:drb] = o
parser.on('-X', '--[no-]drb', 'Run examples via DRb.') do |use_drb|
options[:drb] = use_drb
options[:runner] = RSpec::Core::Invocations::DRbWithFallback.new if use_drb
end

parser.on('--drb-port PORT', 'Port to connect to the DRb server.') do |o|
options[:drb_port] = o.to_i
end

parser.on('--init', 'Initialize your project with RSpec.') do |_cmd|
initialize_project_and_exit
options[:runner] = RSpec::Core::Invocations::InitializeProject.new
end

parser.separator("\n **** Output ****\n\n")
Expand Down Expand Up @@ -242,7 +245,7 @@ def parser(options)
parser.separator("\n **** Utility ****\n\n")

parser.on('-v', '--version', 'Display the version.') do
print_version_and_exit
options[:runner] = RSpec::Core::Invocations::PrintVersion.new
end

# These options would otherwise be confusing to users, so we forcibly
Expand All @@ -254,7 +257,7 @@ def parser(options)
invalid_options = %w[-d --I]

parser.on_tail('-h', '--help', "You're looking at it.") do
print_help_and_exit(parser, invalid_options)
options[:runner] = RSpec::Core::Invocations::PrintHelp.new(parser, invalid_options)
end

# This prevents usage of the invalid_options.
Expand All @@ -268,6 +271,7 @@ def parser(options)
# rubocop:enable Metrics/AbcSize
# rubocop:enable MethodLength
# rubocop:enable CyclomaticComplexity
# rubocop:enable PerceivedComplexity

def add_tag_filter(options, filter_type, tag_name, value=true)
(options[filter_type] ||= {})[tag_name] = value
Expand All @@ -281,39 +285,5 @@ def configure_only_failures(options)
options[:only_failures] = true
add_tag_filter(options, :inclusion_filter, :last_run_status, 'failed')
end

def initialize_project_and_exit
RSpec::Support.require_rspec_core "project_initializer"
ProjectInitializer.new.run
exit
end

def bisect_and_exit(argument)
RSpec::Support.require_rspec_core "bisect/coordinator"

success = Bisect::Coordinator.bisect_with(
original_args,
RSpec.configuration,
bisect_formatter_for(argument)
)

exit(success ? 0 : 1)
end

def bisect_formatter_for(argument)
return Formatters::BisectDebugFormatter if argument == "verbose"
Formatters::BisectProgressFormatter
end

def print_version_and_exit
puts RSpec::Core::Version::STRING
exit
end

def print_help_and_exit(parser, invalid_options)
# Removing the blank invalid options from the output.
puts parser.to_s.gsub(/^\s+(#{invalid_options.join('|')})\s*$\n/, '')
exit
end
end
end
14 changes: 4 additions & 10 deletions lib/rspec/core/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,11 @@ def self.run(args, err=$stderr, out=$stdout)
trap_interrupt
options = ConfigurationOptions.new(args)

if options.options[:drb]
require 'rspec/core/drb'
begin
DRbRunner.new(options).run(err, out)
return
rescue DRb::DRbConnError
err.puts "No DRb server is running. Running in local process instead ..."
end
if options.options[:runner]
options.options[:runner].call(options, err, out)
else
new(options).run(err, out)
end

new(options).run(err, out)
end

def initialize(options, configuration=RSpec.configuration, world=RSpec.world)
Expand Down
5 changes: 2 additions & 3 deletions spec/integration/bisect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ module RSpec::Core

def bisect(cli_args, expected_status=nil)
RSpec.configuration.output_stream = formatter_output
parser = Parser.new(cli_args + ["--bisect"])
expect(parser).to receive(:exit).with(expected_status) if expected_status

expect {
parser.parse
status = RSpec::Core::Runner.run(cli_args + ["--bisect"])
expect(status).to eq(expected_status) if expected_status
}.to avoid_outputting.to_stdout_from_any_process.and avoid_outputting.to_stderr_from_any_process

normalize_durations(formatter_output.string)
Expand Down
64 changes: 60 additions & 4 deletions spec/rspec/core/bisect/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,75 @@ def repro_command_from(ids)
open3_method = Open3.respond_to?(:capture2e) ? :capture2e : :popen3
open3_method = :popen3 if RSpec::Support::Ruby.jruby?

def called_environment
@called_environment
end

if open3_method == :capture2e
RSpec::Matchers.define :invoke_command_with_env do |command, environment|
match do |block|
block.call

expect(Open3).to have_received(open3_method).with(environment, command)
end

supports_block_expectations
end
elsif open3_method == :popen3
RSpec::Matchers.define :invoke_command_with_env do |command, environment|
match do |block|
block.call

expect(Open3).to have_received(open3_method).with(command)
expect(called_environment).to include(environment)
end

supports_block_expectations
end
Copy link
Member

Choose a reason for hiding this comment

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

This can just be supports_block_expectations since you are in a Matchers.define block.

end
Copy link
Member

Choose a reason for hiding this comment

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

Not a merge blocker, but I think maybe the matcher should be named invoke_command_with_env instead of the past-tense have_invoked_command_with_env. Consider that all our other block matchers are named with the present tense form (e.g. change over have_changed, raise_error over have_raised_error, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh - yeah, that's an artifact of switching to the non-block style and back again, will change it now.


before do
allow(Open3).to receive(open3_method).and_return(
allow(Open3).to receive(open3_method) do
@called_environment = ENV.to_hash.dup
[double("Exit Status"), double("Stdout/err")]
)
end

allow(server).to receive(:capture_run_results) do |&block|
block.call
"the results"
end
end

it "runs the suite with the original CLI options" do
runner.original_results
expect(Open3).to have_received(open3_method).with(a_string_including("--seed 1234"))
expect {
runner.original_results
}.to invoke_command_with_env(a_string_including("--seed 1234"), {})
end

context 'when --bisect is present in SPEC_OPTS' do
it "runs the suite with --bisect removed from the environment" do
Copy link
Contributor

Choose a reason for hiding this comment

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

you can test the effect of removing --bisect from the SPEC_OPTS separately from modifying the SPEC_OPTS when invoking the command , if you wish, using something like:

with_env_var 'SPEC_OPTS' => '--bisect --fail-fast' do
  expect(@runner.spec_opts_without_bisect).to eq('--fail-fast')
end
# and then
with_env_vars 'RSPEC_TEMP' => 'original' do
  actual = @runner.run_command "ruby -e 'puts ENV['RSPEC_TEMP']"
  expect(actual).to eq('original')
end

expect {
with_env_vars 'SPEC_OPTS' => '--bisect --fail-fast' do
runner.original_results
end
}.to invoke_command_with_env(
a_string_including("--seed 1234"),
{ 'SPEC_OPTS' => '--fail-fast' }
)
end
end

context 'when --bisect=verbose is present in SPEC_OPTS' do
it "runs the suite with --bisect removed from the environment" do
expect {
with_env_vars 'SPEC_OPTS' => '--bisect=verbose --fail-fast' do
runner.original_results
end
}.to invoke_command_with_env(
a_string_including("--seed 1234"),
{ 'SPEC_OPTS' => '--fail-fast' }
)
end
Copy link
Member

Choose a reason for hiding this comment

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

I see what you mean about it being tricky to figure out what to treat as the subject for these expectations. Maybe the block approach does make the most sense. I'm happy to merge with this has it is but if you want to switch back to the block form that would be fine, too.

end

it 'returns the run results' do
Expand Down
Loading