Skip to content

Commit bd6fa97

Browse files
authored
Introduce CodeOwnership#first_owned_file_for_backtrace (#30)
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.
1 parent 2e43d78 commit bd6fa97

4 files changed

Lines changed: 65 additions & 27 deletions

File tree

Gemfile.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: .
33
specs:
4-
code_ownership (1.29.2)
4+
code_ownership (1.29.3)
55
code_teams (~> 1.0)
66
packs
77
sorbet-runtime
@@ -15,7 +15,7 @@ GEM
1515
coderay (1.1.3)
1616
diff-lcs (1.4.4)
1717
method_source (1.0.0)
18-
packs (0.0.5)
18+
packs (0.0.6)
1919
sorbet-runtime
2020
parser (3.1.2.0)
2121
ast (~> 2.4.1)

code_ownership.gemspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Gem::Specification.new do |spec|
22
spec.name = "code_ownership"
3-
spec.version = '1.29.2'
3+
spec.version = '1.29.3'
44
spec.authors = ['Gusto Engineers']
55
spec.email = ['dev@gusto.com']
66
spec.summary = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership.rb

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ def for_team(team)
5858
ownership_information += ownership_for_mapper
5959
end
6060

61-
6261
ownership_information << ""
6362
end
6463

@@ -93,7 +92,25 @@ def validate!(
9392
# first line that corresponds to a file with assigned ownership
9493
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable(::CodeTeams::Team)) }
9594
def for_backtrace(backtrace, excluded_teams: [])
96-
return unless backtrace
95+
first_owned_file_for_backtrace(backtrace, excluded_teams: excluded_teams)&.first
96+
end
97+
98+
# Given a backtrace from either `Exception#backtrace` or `caller`, find the
99+
# first owned file in it, useful for figuring out which file is being blamed.
100+
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable([::CodeTeams::Team, String])) }
101+
def first_owned_file_for_backtrace(backtrace, excluded_teams: [])
102+
backtrace_with_ownership(backtrace).each do |(team, file)|
103+
if team && !excluded_teams.include?(team)
104+
return [team, file]
105+
end
106+
end
107+
108+
nil
109+
end
110+
111+
sig { params(backtrace: T.nilable(T::Array[String])).returns(T::Enumerable[[T.nilable(::CodeTeams::Team), String]]) }
112+
def backtrace_with_ownership(backtrace)
113+
return [] unless backtrace
97114

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

113-
backtrace.each do |line|
130+
backtrace.lazy.filter_map do |line|
114131
match = line.match(backtrace_line)
132+
next unless match
115133

116-
if match
117-
team = CodeOwnership.for_file(T.must(match[:file]))
118-
if team && !excluded_teams.include?(team)
119-
return team
120-
end
121-
end
134+
file = T.must(match[:file])
135+
136+
[
137+
CodeOwnership.for_file(file),
138+
file,
139+
]
122140
end
123-
nil
124141
end
142+
private_class_method(:backtrace_with_ownership)
125143

126144
sig { params(klass: T.nilable(T.any(Class, Module))).returns(T.nilable(::CodeTeams::Team)) }
127145
def for_class(klass)

spec/lib/code_ownership_spec.rb

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -705,40 +705,60 @@
705705
end
706706

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

718712
context 'excluded_teams is not passed in as an input parameter' do
719713
it 'finds the right team' do
720-
begin # rubocop:disable Style/RedundantBegin
721-
MyFile.raise_error
722-
prevent_false_positive!
723-
rescue StandardError => ex
714+
expect { MyFile.raise_error }.to raise_error do |ex|
724715
expect(CodeOwnership.for_backtrace(ex.backtrace)).to eq CodeTeams.find('Bar')
725716
end
726717
end
727718
end
728719

729720
context 'excluded_teams is passed in as an input parameter' do
730721
it 'ignores the first part of the stack trace and finds the next viable owner' do
731-
begin # rubocop:disable Style/RedundantBegin
732-
MyFile.raise_error
733-
prevent_false_positive!
734-
rescue StandardError => ex
722+
expect { MyFile.raise_error }.to raise_error do |ex|
735723
team_to_exclude = CodeTeams.find('Bar')
736724
expect(CodeOwnership.for_backtrace(ex.backtrace, excluded_teams: [team_to_exclude])).to eq CodeTeams.find('Foo')
737725
end
738726
end
739727
end
740728
end
741729

730+
describe '.first_owned_file_for_backtrace' do
731+
before do
732+
create_files_with_defined_classe
733+
end
734+
735+
736+
context 'excluded_teams is not passed in as an input parameter' do
737+
it 'finds the right team' do
738+
expect { MyFile.raise_error }.to raise_error do |ex|
739+
expect(CodeOwnership.first_owned_file_for_backtrace(ex.backtrace)).to eq [CodeTeams.find('Bar'), 'app/my_error.rb']
740+
end
741+
end
742+
end
743+
744+
context 'excluded_teams is not passed in as an input parameter' do
745+
it 'finds the right team' do
746+
expect { MyFile.raise_error }.to raise_error do |ex|
747+
team_to_exclude = CodeTeams.find('Bar')
748+
expect(CodeOwnership.first_owned_file_for_backtrace(ex.backtrace, excluded_teams: [team_to_exclude])).to eq [CodeTeams.find('Foo'), 'app/my_file.rb']
749+
end
750+
end
751+
end
752+
753+
context 'when nothing is owned' do
754+
it 'returns nil' do
755+
expect { raise 'opsy' }.to raise_error do |ex|
756+
expect(CodeOwnership.first_owned_file_for_backtrace(ex.backtrace)).to be_nil
757+
end
758+
end
759+
end
760+
end
761+
742762
describe '.for_class' do
743763
before { create_files_with_defined_classe }
744764

0 commit comments

Comments
 (0)