Skip to content

Commit

Permalink
Remove inconsistent #order and #randomize? config methods.
Browse files Browse the repository at this point in the history
* Examples and groups can be ordered differently.
* Individual groups can be ordered differently.

As such, a global `order` reader method doesn't
make a lot of sense (although, assigning the global
default via `#order=` still does, I think).

Also:

* This exposed that the ordering force logic
  (e.g. to ensure CLI `--order rand` takes
  precedence over `RSpec.configure`) didn't
  work properly since it simply ensured `order`
  returned the forced option but did not ensure
  the global ordering was set properly.  I've
  fixed this.
* Don't clear the seed when setting the order to
  default. An individual example group may still
  be run in random order and will need the seed.
  There's really no reason to clear it.
* Base our decision of whether or not to print
  the seed at the end of the run on whether or
  not the random strategy ever got used, rather
  than whether or not `random` is the global
  strategy. If only one example group used the
  random strategy we would want it to print,
  but the old logic would not have done so.
  • Loading branch information
myronmarston committed Sep 12, 2013
1 parent 3272be3 commit c59466b
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 165 deletions.
4 changes: 2 additions & 2 deletions features/command_line/require_option.feature
Expand Up @@ -20,8 +20,8 @@ Feature: --require option
super
end
def puts(message)
[@file, __getobj__].each { |out| out.puts message }
def puts(*args)
[@file, __getobj__].each { |out| out.puts(*args) }
end
def close
Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/core/command_line.rb
Expand Up @@ -22,7 +22,7 @@ def run(err, out)
@configuration.load_spec_files
@world.announce_filters

@configuration.reporter.report(@world.example_count, @configuration.randomize? ? @configuration.seed : nil) do |reporter|
@configuration.reporter.report(@world.example_count, @configuration.seed) do |reporter|
begin
@configuration.run_hook(:before, :suite)
@world.ordered_example_groups.map {|g| g.run(reporter) }.all? ? 0 : @configuration.failure_exit_code
Expand Down
51 changes: 22 additions & 29 deletions lib/rspec/core/configuration.rb
Expand Up @@ -108,10 +108,6 @@ def self.add_setting(name, opts={})
# The exit code to return if there are any failures (default: 1).
add_setting :failure_exit_code

# Determines the order in which examples are run (default: OS standard
# load order for files, declaration order for groups and examples).
define_reader :order

# Indicates files configured to be required
define_reader :requires

Expand Down Expand Up @@ -194,7 +190,6 @@ def initialize
@mock_framework = nil
@files_to_run = []
@formatters = []
@order = "default"
@color = false
@pattern = '**/*_spec.rb'
@failure_exit_code = 1
Expand Down Expand Up @@ -295,6 +290,11 @@ def mock_framework=(framework)
mock_with framework
end

def seed_used?
example_ordering_registry.used_random_seed? ||
group_ordering_registry.used_random_seed?
end

# The patterns to always include to backtraces.
#
# Defaults to [Regexp.new Dir.getwd] if the current working directory
Expand Down Expand Up @@ -878,23 +878,18 @@ def format_docstrings_block

# @api
#
# Sets the seed value and sets `order='rand'`
# Sets the seed value and sets the default global ordering to random.
def seed=(seed)
order_and_seed_from_seed(seed)
end

# @api
#
# Sets the order and, if order is `'rand:<seed>'`, also sets the seed.
# Sets the default global order and, if order is `'rand:<seed>'`, also sets the seed.
def order=(type)
order_and_seed_from_order(type)
end

def randomize?
order.to_s.match(/rand/)
end


# Sets a strategy by which to order examples.
#
# @example
Expand All @@ -909,8 +904,9 @@ def randomize?
# @see #order=
# @see #seed=
def order_examples(ordering=nil, &block)
strategy = @example_ordering_registry.set_global_order(ordering, &block)
@order = "custom" unless strategy.built_in?
unless @preferred_options.has_key?(:order)
@example_ordering_registry.set_global_order(ordering, &block)
end
end

# Sets a strategy by which to order groups.
Expand All @@ -927,8 +923,9 @@ def order_examples(ordering=nil, &block)
# @see #order=
# @see #seed=
def order_groups(ordering=nil, &block)
strategy = @group_ordering_registry.set_global_order(ordering, &block)
@order = "custom" unless strategy.built_in?
unless @preferred_options.has_key?(:order)
@group_ordering_registry.set_global_order(ordering, &block)
end
end

# Sets a strategy by which to order groups and examples.
Expand Down Expand Up @@ -1077,11 +1074,10 @@ def file_at(path)
end

def order_and_seed_from_seed(value)
@example_ordering_registry.set_global_order(:random)
@group_ordering_registry.set_global_order(:random)
order_groups_and_examples(:random)

@order, @seed = 'rand', value.to_i
[@order, @seed]
order, @seed = 'rand', value.to_i
[order, @seed]
end

def set_order_and_seed(hash)
Expand All @@ -1091,19 +1087,16 @@ def set_order_and_seed(hash)

def order_and_seed_from_order(type)
order, seed = type.to_s.split(':')
@order = order
@seed = seed = seed.to_i if seed
@seed = seed = seed.to_i if seed

if randomize?
@example_ordering_registry.set_global_order(:random)
@group_ordering_registry.set_global_order(:random)
ordering_name = if order.include?('rand')
:random
elsif order == 'default'
@order, @seed = nil, nil

@example_ordering_registry.set_global_order(:default)
@group_ordering_registry.set_global_order(:default)
:default
end

order_groups_and_examples(ordering_name) if ordering_name

return order, seed
end
end
Expand Down
22 changes: 10 additions & 12 deletions lib/rspec/core/ordering.rb
Expand Up @@ -5,27 +5,25 @@ class Identity
def order(items)
items
end

def built_in?
true
end
end

class Random
def initialize(configuration)
@configuration = configuration
@used = false
end

def used?
@used
end

def order(items)
@used = true
Kernel.srand @configuration.seed
ordering = items.shuffle
Kernel.srand # reset random generation
ordering
end

def built_in?
true
end
end

class Custom
Expand All @@ -36,10 +34,6 @@ def initialize(callable)
def order(list)
@callable.call(list)
end

def built_in?
false
end
end

class Registry
Expand Down Expand Up @@ -76,6 +70,10 @@ def set_global_order(name = nil, &block)
@global_ordering = @strategies.fetch(name)
end
end

def used_random_seed?
@strategies[:random].used?
end
end
end
end
Expand Down
8 changes: 7 additions & 1 deletion lib/rspec/core/reporter.rb
Expand Up @@ -109,7 +109,7 @@ def finish(seed)
notify :dump_failures
notify :dump_summary, @duration, @example_count, @failure_count, @pending_count
notify :deprecation_summary
notify :seed, seed if seed
notify :seed, seed if seed && seed_used?
ensure
notify :close
end
Expand All @@ -127,5 +127,11 @@ def notify(event, *args, &block)
formatter.send(event, *args, &block)
end
end

private

def seed_used?
RSpec.configuration.seed_used?
end
end
end
110 changes: 59 additions & 51 deletions spec/rspec/core/configuration_spec.rb
Expand Up @@ -1312,24 +1312,46 @@ def metadata_hash(*args)
end

describe "#force" do
it "forces order" do
config.force :order => "default"
config.order = "rand"
expect(config.order).to eq("default")
end
context "for ordering options" do
let(:list) { [1, 2, 3, 4] }
let(:example_ordering_strategy) { config.example_ordering_registry.global_ordering }
let(:group_ordering_strategy) { config.group_ordering_registry.global_ordering }

it "forces order and seed with :order => 'rand:37'" do
config.force :order => "rand:37"
config.order = "default"
expect(config.order).to eq("rand")
expect(config.seed).to eq(37)
end
let(:shuffled) do
Kernel.srand(37)
list.shuffle
end

specify "CLI `--order default` takes precedence over `config.order = rand`" do
config.force :order => "default"
config.order = "rand"

expect(example_ordering_strategy.order(list)).to eq([1, 2, 3, 4])
end

specify "CLI `--order rand:37` takes precedence over `config.order = default`" do
config.force :order => "rand:37"
config.order = "default"

expect(example_ordering_strategy.order(list)).to eq(shuffled)
end

specify "CLI `--seed 37` forces order and seed" do
config.force :seed => 37
config.order = "default"
config.seed = 145

expect(example_ordering_strategy.order(list)).to eq(shuffled)
expect(config.seed).to eq(37)
end

it "forces order and seed with :seed => '37'" do
config.force :seed => "37"
config.order = "default"
expect(config.seed).to eq(37)
expect(config.order).to eq("rand")
specify "CLI `--order default` takes precedence over `config.order_groups_and_examples`" do
config.force :order => "default"
config.order_groups_and_examples(&:reverse)

expect(example_ordering_strategy.order(list)).to eq([1, 2, 3, 4])
expect(group_ordering_strategy.order(list)).to eq([1, 2, 3, 4])
end
end

it "forces 'false' value" do
Expand All @@ -1350,21 +1372,29 @@ def metadata_hash(*args)
end
end

describe '#randomize?' do
context 'with order set to :random' do
before { config.order = :random }
describe "#seed_used?" do
def use_seed_on(registry)
registry[:random].order([1, 2])
end

it 'returns true' do
expect(config.randomize?).to be_truthy
end
it 'returns false if neither ordering registry used the seed' do
expect(config.seed_used?).to be false
end

context 'with order set to nil' do
before { config.order = nil }
it 'returns true if the example ordering registry used the seed' do
use_seed_on(config.example_ordering_registry)
expect(config.seed_used?).to be true
end

it 'returns false' do
expect(config.randomize?).to be_falsey
end
it 'returns true if the group ordering registry used the seed' do
use_seed_on(config.group_ordering_registry)
expect(config.seed_used?).to be true
end

it 'returns true if both ordering registries used the seed' do
use_seed_on(config.example_ordering_registry)
use_seed_on(config.group_ordering_registry)
expect(config.seed_used?).to be true
end
end

Expand All @@ -1375,10 +1405,6 @@ def metadata_hash(*args)
config.order = 'random'
end

it 'sets order to "random"' do
expect(config.order).to eq('random')
end

it 'does not change the seed' do
expect(config.seed).to eq(7654)
end
Expand All @@ -1393,10 +1419,6 @@ def metadata_hash(*args)
context 'given "random:123"' do
before { config.order = 'random:123' }

it 'sets order to "random"' do
expect(config.order).to eq('random')
end

it 'sets seed to 123' do
expect(config.seed).to eq(123)
end
Expand All @@ -1414,12 +1436,8 @@ def metadata_hash(*args)
config.order = 'default'
end

it "sets the order to nil" do
expect(config.order).to be_nil
end

it "sets the seed to nil" do
expect(config.seed).to be_nil
it "does not change the seed" do
expect(config.seed).to eq(123)
end

it 'clears the random ordering' do
Expand All @@ -1441,11 +1459,6 @@ def metadata_hash(*args)
ordering_strategy = config.example_ordering_registry.global_ordering
expect(ordering_strategy.order(examples)).to eq([4, 3, 2, 1])
end

it 'sets #order to "custom"' do
config.order_examples { |examples| examples.reverse }
expect(config.order).to eq("custom")
end
end

describe "#order_groups" do
Expand All @@ -1457,11 +1470,6 @@ def metadata_hash(*args)
ordering_strategy = config.group_ordering_registry.global_ordering
expect(ordering_strategy.order(groups)).to eq([4, 3, 2, 1])
end

it 'sets #order to "custom"' do
config.order_groups { |groups| groups.reverse }
expect(config.order).to eq("custom")
end
end

describe "#order_groups_and_examples" do
Expand Down

0 comments on commit c59466b

Please sign in to comment.