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

Commit 5eebff7

Browse files
committed
Refactor: make persister easier to reason about.
Before `statuses_from_previous_runs` depended upon the `@previous_runs` ivar, which was set by `persist`, so the method would fail if called before `persist`, and if called after `persist` was done, it would re-use the old `@previous_runs` state. Pretty weird. Far better to pass explicit local variables around.
1 parent cc1a640 commit 5eebff7

File tree

2 files changed

+6
-12
lines changed

2 files changed

+6
-12
lines changed

lib/rspec/core/example_status_persister.rb

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,22 @@ def persist
2727
# condition where parallel or unrelated spec runs race to
2828
# update the same file
2929
f.flock(File::LOCK_EX)
30-
@previous_runs = f.read
30+
unparsed_previous_runs = f.read
3131
f.rewind
32-
f.write(dumped_statuses)
32+
f.write(dump_statuses(unparsed_previous_runs))
3333
f.flush
3434
f.truncate(f.pos)
3535
end
3636
end
3737

3838
private
3939

40-
def dumped_statuses
40+
def dump_statuses(unparsed_previous_runs)
41+
statuses_from_previous_runs = ExampleStatusParser.parse(unparsed_previous_runs)
42+
merged_statuses = ExampleStatusMerger.merge(statuses_from_this_run, statuses_from_previous_runs)
4143
ExampleStatusDumper.dump(merged_statuses)
4244
end
4345

44-
def merged_statuses
45-
ExampleStatusMerger.merge(statuses_from_this_run, statuses_from_previous_runs)
46-
end
47-
4846
def statuses_from_this_run
4947
@examples.map do |ex|
5048
result = ex.execution_result
@@ -56,10 +54,6 @@ def statuses_from_this_run
5654
}
5755
end
5856
end
59-
60-
def statuses_from_previous_runs
61-
ExampleStatusParser.parse(@previous_runs)
62-
end
6357
end
6458

6559
# Merges together a list of example statuses from this run

spec/rspec/core/example_status_persister_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def new_example(id, metadata = {})
7373

7474
# dumped_statuses is called after the file is locked but
7575
# before the output is written
76-
allow(persister_1).to receive(:dumped_statuses).and_wrap_original do |m, *args|
76+
allow(persister_1).to receive(:dump_statuses).and_wrap_original do |m, *args|
7777
persister_2_thread = Thread.new { persister_2.persist }
7878
m.call(*args)
7979
end

0 commit comments

Comments
 (0)