Skip to content

Commit

Permalink
Introduce CodeOwnership#first_owned_file_for_backtrace (#30)
Browse files Browse the repository at this point in the history
The intent is to expose more data and enable a consumer to report additional information. For example, we wish to use this method to log the "blamed" stack of an error.
  • Loading branch information
mzruya authored Feb 2, 2023
1 parent 2e43d78 commit bd6fa97
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 27 deletions.
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
code_ownership (1.29.2)
code_ownership (1.29.3)
code_teams (~> 1.0)
packs
sorbet-runtime
Expand All @@ -15,7 +15,7 @@ GEM
coderay (1.1.3)
diff-lcs (1.4.4)
method_source (1.0.0)
packs (0.0.5)
packs (0.0.6)
sorbet-runtime
parser (3.1.2.0)
ast (~> 2.4.1)
Expand Down
2 changes: 1 addition & 1 deletion code_ownership.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |spec|
spec.name = "code_ownership"
spec.version = '1.29.2'
spec.version = '1.29.3'
spec.authors = ['Gusto Engineers']
spec.email = ['[email protected]']
spec.summary = 'A gem to help engineering teams declare ownership of code'
Expand Down
38 changes: 28 additions & 10 deletions lib/code_ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ def for_team(team)
ownership_information += ownership_for_mapper
end


ownership_information << ""
end

Expand Down Expand Up @@ -93,7 +92,25 @@ def validate!(
# first line that corresponds to a file with assigned ownership
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable(::CodeTeams::Team)) }
def for_backtrace(backtrace, excluded_teams: [])
return unless backtrace
first_owned_file_for_backtrace(backtrace, excluded_teams: excluded_teams)&.first
end

# Given a backtrace from either `Exception#backtrace` or `caller`, find the
# first owned file in it, useful for figuring out which file is being blamed.
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable([::CodeTeams::Team, String])) }
def first_owned_file_for_backtrace(backtrace, excluded_teams: [])
backtrace_with_ownership(backtrace).each do |(team, file)|
if team && !excluded_teams.include?(team)
return [team, file]
end
end

nil
end

sig { params(backtrace: T.nilable(T::Array[String])).returns(T::Enumerable[[T.nilable(::CodeTeams::Team), String]]) }
def backtrace_with_ownership(backtrace)
return [] unless backtrace

# The pattern for a backtrace hasn't changed in forever and is considered
# stable: https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L303-L317
Expand All @@ -110,18 +127,19 @@ def for_backtrace(backtrace, excluded_teams: [])
`(?<function>.*)' # Matches "`block (3 levels) in create'"
\z}x

backtrace.each do |line|
backtrace.lazy.filter_map do |line|
match = line.match(backtrace_line)
next unless match

if match
team = CodeOwnership.for_file(T.must(match[:file]))
if team && !excluded_teams.include?(team)
return team
end
end
file = T.must(match[:file])

[
CodeOwnership.for_file(file),
file,
]
end
nil
end
private_class_method(:backtrace_with_ownership)

sig { params(klass: T.nilable(T.any(Class, Module))).returns(T.nilable(::CodeTeams::Team)) }
def for_class(klass)
Expand Down
48 changes: 34 additions & 14 deletions spec/lib/code_ownership_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -705,40 +705,60 @@
end

describe '.for_backtrace' do
def prevent_false_positive!
# The above code should raise, and we should never arrive at this next expectation.
# This is just to protect against a case where we have a false-postive test because the above does not raise.
expect(true).to eq false # rubocop:disable RSpec/ExpectActual
end

before do
create_files_with_defined_classe
end

context 'excluded_teams is not passed in as an input parameter' do
it 'finds the right team' do
begin # rubocop:disable Style/RedundantBegin
MyFile.raise_error
prevent_false_positive!
rescue StandardError => ex
expect { MyFile.raise_error }.to raise_error do |ex|
expect(CodeOwnership.for_backtrace(ex.backtrace)).to eq CodeTeams.find('Bar')
end
end
end

context 'excluded_teams is passed in as an input parameter' do
it 'ignores the first part of the stack trace and finds the next viable owner' do
begin # rubocop:disable Style/RedundantBegin
MyFile.raise_error
prevent_false_positive!
rescue StandardError => ex
expect { MyFile.raise_error }.to raise_error do |ex|
team_to_exclude = CodeTeams.find('Bar')
expect(CodeOwnership.for_backtrace(ex.backtrace, excluded_teams: [team_to_exclude])).to eq CodeTeams.find('Foo')
end
end
end
end

describe '.first_owned_file_for_backtrace' do
before do
create_files_with_defined_classe
end


context 'excluded_teams is not passed in as an input parameter' do
it 'finds the right team' do
expect { MyFile.raise_error }.to raise_error do |ex|
expect(CodeOwnership.first_owned_file_for_backtrace(ex.backtrace)).to eq [CodeTeams.find('Bar'), 'app/my_error.rb']
end
end
end

context 'excluded_teams is not passed in as an input parameter' do
it 'finds the right team' do
expect { MyFile.raise_error }.to raise_error do |ex|
team_to_exclude = CodeTeams.find('Bar')
expect(CodeOwnership.first_owned_file_for_backtrace(ex.backtrace, excluded_teams: [team_to_exclude])).to eq [CodeTeams.find('Foo'), 'app/my_file.rb']
end
end
end

context 'when nothing is owned' do
it 'returns nil' do
expect { raise 'opsy' }.to raise_error do |ex|
expect(CodeOwnership.first_owned_file_for_backtrace(ex.backtrace)).to be_nil
end
end
end
end

describe '.for_class' do
before { create_files_with_defined_classe }

Expand Down

0 comments on commit bd6fa97

Please sign in to comment.