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

WIP Refactor group profile to include beforehook #1942

Closed
wants to merge 18 commits into from
Closed
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
22 changes: 22 additions & 0 deletions features/configuration/profile.feature
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,25 @@ Feature: Profile examples
When I run `rspec spec --fail-fast --profile`
Then the output should not contain "Top 2 slowest examples"
And the output should not contain "example 1"

Scenario: Example groups profiling include before hook execution time
Given a file named "spec/example_spec.rb" with:
"""ruby
RSpec.describe "slow before context hook" do
before(:context) do
sleep 0.2
end
it "example" do
expect(10).to eq(10)
end
end

RSpec.describe "slow example" do
it "slow example" do
sleep 0.1
expect(10).to eq(10)
end
end
"""
When I run `rspec spec --profile 1`
Then the output should contain "slow before context hook"
24 changes: 22 additions & 2 deletions lib/rspec/core/formatters/json_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,36 @@ module Core
module Formatters
# @private
class JsonFormatter < BaseFormatter
Formatters.register self, :message, :dump_summary, :dump_profile, :stop, :close
Formatters.register self, :message, :dump_summary, :dump_profile, :stop,
:close, :example_group_started, :example_group_finished,
:example_started

attr_reader :output_hash

def initialize(output)
super
@example_groups= {} #todo rename, maybe example_groups_data ...
@output_hash = {
:version => RSpec::Core::Version::STRING
}
end

#todo remove duplication with lib/rspec/core/formatters/profile_formatter.rb line 16
def example_group_started(notification)
@example_groups[notification.group.id] = Hash.new(0)
@example_groups[notification.group.id][:start] = Time.now
@example_groups[notification.group.id][:description] = notification.group.top_level_description
end

def example_group_finished(notification)
@example_groups[notification.group.id][:total_time] = Time.now - @example_groups[notification.group.id][:start]
end

def example_started(notification)
group = notification.example.example_group.parent_groups.last.id
@example_groups[group][:count] += 1
end

def message(notification)
(@output_hash[:messages] ||= []) << notification.message
end
Expand Down Expand Up @@ -72,8 +91,9 @@ def dump_profile_slowest_examples(profile)

# @api private
def dump_profile_slowest_example_groups(profile)
slowest_groups = profile.calculate_slowest_groups(@example_groups)
@output_hash[:profile] ||= {}
@output_hash[:profile][:groups] = profile.slowest_groups.map do |loc, hash|
@output_hash[:profile][:groups] = slowest_groups.map do |loc, hash|
hash.update(:location => loc)
end
end
Expand Down
30 changes: 26 additions & 4 deletions lib/rspec/core/formatters/profile_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,36 @@ module Formatters
# @api private
# Formatter for providing profile output.
class ProfileFormatter
Formatters.register self, :dump_profile
Formatters.register self, :dump_profile, :example_group_started, :example_group_finished, :example_started

def initialize(output)
@example_groups = {} #todo rename, maybe groups_data, groups_information or profile_information
@output = output
end

# @private
attr_reader :output

# @private
def example_group_started(notification)
@example_groups[notification.group.id] = Hash.new(0)
@example_groups[notification.group.id][:start] = Time.now
@example_groups[notification.group.id][:description] = notification.group.top_level_description
end

# @private
def example_group_finished(notification)
@example_groups[notification.group.id][:total_time] = Time.now - @example_groups[notification.group.id][:start]
Copy link
Member

Choose a reason for hiding this comment

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

The logic you've added to these methods should be moved elsewhere, either to the notifications or to the reporter that generates them so that we're not repeating the logic (and so eventually other formatters can make use of it if they want to)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I find how to register to these events on the ProfileNotification, I start a new branch where I don't need to change to the formatter.

end

# @private
def example_started(notification)
#todo: maybe move example_group.parent_groups.last to an example or notification method like example.last_anscestor_group
group = notification.example.example_group.parent_groups.last.id
@example_groups[group][:count] += 1
end


# @api public
#
# This method is invoked after the dumping the summary if profiling is
Expand Down Expand Up @@ -42,10 +63,11 @@ def dump_profile_slowest_examples(profile)
end

def dump_profile_slowest_example_groups(profile)
return if profile.slowest_groups.empty?
slowest_groups = profile.calculate_slowest_groups(@example_groups)
return if slowest_groups.empty?

@output.puts "\nTop #{profile.slowest_groups.size} slowest example groups:"
profile.slowest_groups.each do |loc, hash|
@output.puts "\nTop #{slowest_groups.size} slowest example groups:"
slowest_groups.each do |loc, hash|
average = "#{bold(Helpers.format_seconds(hash[:average]))} #{bold("seconds")} average"
total = "#{Helpers.format_seconds(hash[:total_time])} seconds"
count = Helpers.pluralize(hash[:count], "example")
Expand Down
21 changes: 3 additions & 18 deletions lib/rspec/core/notifications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -577,24 +577,8 @@ def percentage
end

# @return [Array<RSpec::Core::Example>] the slowest example groups
def slowest_groups
@slowest_groups ||= calculate_slowest_groups
end

private

def calculate_slowest_groups
example_groups = {}

examples.each do |example|
location = example.example_group.parent_groups.last.metadata[:location]

location_hash = example_groups[location] ||= Hash.new(0)
location_hash[:total_time] += example.execution_result.run_time
location_hash[:count] += 1
next if location_hash.key?(:description)
location_hash[:description] = example.example_group.top_level_description
end
# todo rename this method or move it's code to the json_formater.rb and profile.formater.rb
Copy link
Member

Choose a reason for hiding this comment

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

This code shouldn't be moved to the formatters, it should be de-duplicated here.

def calculate_slowest_groups(example_groups)

# stop if we've only one example group
return {} if example_groups.keys.length <= 1
Expand All @@ -607,6 +591,7 @@ def calculate_slowest_groups
end
end


# The `DeprecationNotification` is issued by the reporter when a deprecated
# part of RSpec is encountered. It represents information about the
# deprecated call site.
Expand Down
6 changes: 4 additions & 2 deletions spec/rspec/core/formatters/json_formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,12 @@ def profile *groups
end

it "provides information" do
expect(formatter.output_hash[:profile][:groups].first.keys).to match_array([:total_time, :count, :description, :average, :location])
expect(formatter.output_hash[:profile][:groups].first.keys).to match_array([:total_time, :count, :description, :average, :location, :start])
end

it "ranks the example groups by average time" do
#todo the group profiling dont use the example clock anymore
#I need to adapt this test
xit "ranks the example groups by average time" do
expect(formatter.output_hash[:profile][:groups].first[:description]).to eq("slow group")
end
end
Expand Down