-
-
Notifications
You must be signed in to change notification settings - Fork 753
Refactor group profile to include before hooks #1971
Changes from all commits
c7cf688
a0dc34b
c29fac9
cb0b34e
6acd72c
1e175e3
4ee7e36
85cb4ee
2e59a6e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
module RSpec | ||
module Core | ||
# @private | ||
class Profiler | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this really only provides profiling for example groups (since each individual example profiles itself), do you think it should be called On a side note, I think I'd prefer to see this in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm strongly against polluting the formatters namespace with more things that are not formatters. The reporter is not in the formatters namespace ergo this isn't. |
||
NOTIFICATIONS = [:example_group_started, :example_group_finished, :example_started] | ||
|
||
def initialize | ||
@example_groups = Hash.new { |h, k| h[k] = { :count => 0 } } | ||
end | ||
|
||
attr_reader :example_groups | ||
|
||
def example_group_started(notification) | ||
@example_groups[notification.group][:start] = Time.now | ||
@example_groups[notification.group][:description] = notification.group.top_level_description | ||
end | ||
|
||
def example_group_finished(notification) | ||
@example_groups[notification.group][:total_time] = Time.now - @example_groups[notification.group][:start] | ||
end | ||
|
||
def example_started(notification) | ||
group = notification.example.example_group.parent_groups.last | ||
@example_groups[group][:count] += 1 | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
require 'rspec/core/profiler' | ||
|
||
RSpec.describe 'RSpec::Core::Profiler' do | ||
let(:profiler) { RSpec::Core::Profiler.new } | ||
|
||
it 'starts with an empty hash of example_groups' do | ||
expect(profiler.example_groups).to be_empty.and be_a Hash | ||
end | ||
|
||
context 'when hooked into the reporter' do | ||
include FormatterSupport | ||
|
||
let(:description ) { "My Group" } | ||
let(:now) { ::Time.utc(2015, 6, 2) } | ||
|
||
before do | ||
allow(::RSpec::Core::Time).to receive(:now) { now } | ||
end | ||
|
||
def group | ||
@group ||= | ||
begin | ||
group = super | ||
allow(group).to receive(:top_level_description) { description } | ||
group | ||
end | ||
end | ||
|
||
describe '#example_group_started' do | ||
it 'records example groups start time and description via id' do | ||
expect { | ||
profiler.example_group_started group_notification group | ||
}.to change { profiler.example_groups[group] }. | ||
from(a_hash_excluding(:start, :description)). | ||
to(a_hash_including(:start => now, :description => description)) | ||
end | ||
end | ||
|
||
describe '#example_group_finished' do | ||
before do | ||
profiler.example_group_started group_notification group | ||
allow(::RSpec::Core::Time).to receive(:now) { now + 1 } | ||
end | ||
|
||
it 'records example groups total time and description via id' do | ||
expect { | ||
profiler.example_group_finished group_notification group | ||
}.to change { profiler.example_groups[group] }. | ||
from(a_hash_excluding(:total_time)). | ||
to(a_hash_including(:total_time => 1.0)) | ||
end | ||
end | ||
|
||
describe '#example_started' do | ||
let(:example) { new_example } | ||
before do | ||
allow(example).to receive(:example_group) { group } | ||
allow(group).to receive(:parent_groups) { [group] } | ||
profiler.example_group_started group_notification group | ||
end | ||
|
||
it 'increments the count of examples for its parent group' do | ||
expect { | ||
profiler.example_started example_notification example | ||
}.to change { profiler.example_groups[group][:count] }.by 1 | ||
end | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to this PR or just a refactoring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well we're not sorting the examples here so why assign a local variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine. I was just making sure there wasn't some behavioral change here I wasn't noticing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, just clean up :)