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

Commit 7584e59

Browse files
committed
Use "unknown" for :last_run_status when we don't have a value.
This ensures that the value is consistent — before for an unknown case it could either be `nil` or `"unknown"`. We've also moved the constant into configuration, so that we can avoid the cost of loading `example_status_persister` (via constant access since it is setup to be autoloaded) if the user hasn't configured the feature.
1 parent 65e5a03 commit 7584e59

File tree

4 files changed

+26
-14
lines changed

4 files changed

+26
-14
lines changed

lib/rspec/core/configuration.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -834,17 +834,19 @@ def files_to_run
834834

835835
# @private
836836
def last_run_statuses
837-
@last_run_statuses ||=
837+
@last_run_statuses ||= Hash.new(UNKNOWN_STATUS).tap do |statuses|
838838
if (path = example_status_persistence_file_path)
839-
ExampleStatusPersister.load_from(path).inject({}) do |hash, example|
839+
ExampleStatusPersister.load_from(path).inject(statuses) do |hash, example|
840840
hash[example.fetch(:example_id)] = example.fetch(:status)
841841
hash
842842
end
843-
else
844-
{}
845843
end
844+
end
846845
end
847846

847+
# @private
848+
UNKNOWN_STATUS = "unknown".freeze
849+
848850
# @private
849851
FAILED_STATUS = "failed".freeze
850852

lib/rspec/core/example_status_persister.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def statuses_from_this_run
4545

4646
{
4747
:example_id => ex.id,
48-
:status => result.status ? result.status.to_s : ExampleStatusMerger::UNKNOWN_STATUS,
48+
:status => result.status ? result.status.to_s : Configuration::UNKNOWN_STATUS,
4949
:run_time => result.run_time ? Formatters::Helpers.format_duration(result.run_time) : ""
5050
}
5151
end
@@ -86,12 +86,10 @@ def merge
8686
delete_previous_examples_that_no_longer_exist
8787

8888
@this_run.merge(@from_previous_runs) do |_ex_id, new, old|
89-
new.fetch(:status) == UNKNOWN_STATUS ? old : new
89+
new.fetch(:status) == Configuration::UNKNOWN_STATUS ? old : new
9090
end.values.sort_by(&method(:sort_value_from))
9191
end
9292

93-
UNKNOWN_STATUS = "unknown".freeze
94-
9593
private
9694

9795
def hash_from(example_list)

spec/rspec/core/configuration/only_failures_support_spec.rb

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,33 @@ def last_run_statuses
3131
it 'returns a memoized value' do
3232
expect(last_run_statuses).to be(last_run_statuses)
3333
end
34+
35+
specify 'the hash returns `unknown` for unknown example ids for consistency' do
36+
expect(last_run_statuses["foo"]).to eq(Configuration::UNKNOWN_STATUS)
37+
expect(last_run_statuses["bar"]).to eq(Configuration::UNKNOWN_STATUS)
38+
end
3439
end
3540

3641
context "when `example_status_persistence_file_path` is not configured" do
42+
before do
43+
config.example_status_persistence_file_path = nil
44+
end
45+
3746
it 'returns a memoized value' do
3847
expect(last_run_statuses).to be(last_run_statuses)
3948
end
4049

4150
it 'returns a blank hash without attempting to load the persisted statuses' do
42-
config.example_status_persistence_file_path = nil
43-
4451
persister = class_double(ExampleStatusPersister).as_stubbed_const
4552
expect(persister).not_to receive(:load_from)
4653

4754
expect(last_run_statuses).to eq({})
4855
end
56+
57+
specify 'the hash returns `unknown` for all ids for consistency' do
58+
expect(last_run_statuses["foo"]).to eq(Configuration::UNKNOWN_STATUS)
59+
expect(last_run_statuses["bar"]).to eq(Configuration::UNKNOWN_STATUS)
60+
end
4961
end
5062

5163
def allows_value_to_change_when_updated

spec/rspec/core/example_status_persister_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,14 @@ def new_example(id, metadata = {})
8282
expect(loaded).to match [ a_hash_including(:run_time => "3 seconds") ]
8383
end
8484

85-
it "persists a loaded but unexecuted example with an #{ExampleStatusMerger::UNKNOWN_STATUS} status" do
85+
it "persists a loaded but unexecuted example with an #{Configuration::UNKNOWN_STATUS} status" do
8686
ex_1 = RSpec.describe.example
8787

8888
ExampleStatusPersister.persist([ex_1], file.path)
8989
loaded = ExampleStatusPersister.load_from(file.path)
9090

9191
expect(loaded).to match [ a_hash_including(
92-
:example_id => ex_1.id, :status => ExampleStatusMerger::UNKNOWN_STATUS
92+
:example_id => ex_1.id, :status => Configuration::UNKNOWN_STATUS
9393
) ]
9494
end
9595

@@ -142,7 +142,7 @@ def new_example(id, metadata = {})
142142

143143
context "for examples that are only in the set for this run" do
144144
it "takes them indiscriminately, even if they did not execute" do
145-
this_run = [ example(existing_spec_file, "1:1", ExampleStatusMerger::UNKNOWN_STATUS) ]
145+
this_run = [ example(existing_spec_file, "1:1", Configuration::UNKNOWN_STATUS) ]
146146

147147
merged = merge(:this_run => this_run, :from_previous_runs => [])
148148
expect(merged).to match_array(this_run)
@@ -187,7 +187,7 @@ def new_example(id, metadata = {})
187187
end
188188

189189
it "takes the status from previous runs if the example was loaded but did not execute" do
190-
this_run = [ example("foo_spec.rb", "1:1", ExampleStatusMerger::UNKNOWN_STATUS) ]
190+
this_run = [ example("foo_spec.rb", "1:1", Configuration::UNKNOWN_STATUS) ]
191191
from_previous_runs = [ example("foo_spec.rb", "1:1", "failed") ]
192192

193193
merged = merge(:this_run => this_run, :from_previous_runs => from_previous_runs)

0 commit comments

Comments
 (0)