Skip to content

Commit 8fddb42

Browse files
perryqhperryqh
committed
allow raise predicate
1 parent c0c8225 commit 8fddb42

7 files changed

Lines changed: 69 additions & 35 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/code_ownership/Cargo.toml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ rb-sys = { version = "0.9.111", features = [
1717
magnus = { version = "0.7.1" }
1818
serde = { version = "1.0.219", features = ["derive"] }
1919
serde_magnus = "0.9.0"
20-
# codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.2.17" }
21-
codeowners = { path = "../../../codeowners-rs" }
20+
codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.2.17" }
2221

2322
[dev-dependencies]
2423
rb-sys = { version = "0.9.117", features = [

‎lib/code_ownership.rb‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -40,26 +40,18 @@ def version
4040
"codeowners-rs version: #{::RustCodeOwners.version}"]
4141
end
4242

43-
# Returns the team that owns the given file based on the CODEOWNERS file.
44-
# This is much faster and can be safely used when the CODEOWNERS files is up to date.
45-
# Examples of reliable usage:
46-
# - running in CI pipeline
47-
# - running on the server
48-
# Examples of unreliable usage:
49-
# - running in IDE when files are changing and the CODEOWNERS file is not getting updated
50-
sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
51-
def for_file_from_codeowners(file)
52-
teams_for_files_from_codeowners([file]).values.first
53-
end
54-
55-
sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
56-
def for_file(file)
57-
Private::TeamFinder.for_file(file)
43+
sig { params(file: String, from_codeowners: T::Boolean, allow_raise: T::Boolean).returns(T.nilable(CodeTeams::Team)) }
44+
def for_file(file, from_codeowners: true, allow_raise: false)
45+
if from_codeowners
46+
teams_for_files_from_codeowners([file], allow_raise: allow_raise).values.first
47+
else
48+
Private::TeamFinder.for_file(file, allow_raise: allow_raise)
49+
end
5850
end
5951

60-
sig { params(files: T::Array[String]).returns(T::Hash[String, T.nilable(CodeTeams::Team)]) }
61-
def teams_for_files_from_codeowners(files)
62-
Private::TeamFinder.teams_for_files(files)
52+
sig { params(files: T::Array[String], allow_raise: T::Boolean).returns(T::Hash[String, T.nilable(CodeTeams::Team)]) }
53+
def teams_for_files_from_codeowners(files, allow_raise: false)
54+
Private::TeamFinder.teams_for_files(files, allow_raise: allow_raise)
6355
end
6456

6557
sig { params(file: String).returns(T.nilable(T::Hash[Symbol, String])) }

‎lib/code_ownership/code_ownership.bundle‎

-76.9 KB
Binary file not shown.

‎lib/code_ownership/private/for_file_output_builder.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def build_verbose
5353

5454
sig { returns(T::Hash[Symbol, T.untyped]) }
5555
def build_terse
56-
team = CodeOwnership.for_file(@file_path)
56+
team = CodeOwnership.for_file(@file_path, from_codeowners: false, allow_raise: true)
5757

5858
if team.nil?
5959
UNOWNED_OUTPUT

‎lib/code_ownership/private/team_finder.rb‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ module TeamFinder
1212

1313
requires_ancestor { Kernel }
1414

15-
sig { params(file_path: String).returns(T.nilable(CodeTeams::Team)) }
16-
def for_file(file_path)
15+
sig { params(file_path: String, allow_raise: T::Boolean).returns(T.nilable(CodeTeams::Team)) }
16+
def for_file(file_path, allow_raise: false)
1717
return nil if file_path.start_with?('./')
1818

1919
return FilePathTeamCache.get(file_path) if FilePathTeamCache.cached?(file_path)
@@ -24,17 +24,17 @@ def for_file(file_path)
2424
if result[:team_name].nil?
2525
FilePathTeamCache.set(file_path, nil)
2626
else
27-
FilePathTeamCache.set(file_path, T.let(find_team!(T.must(result[:team_name])), T.nilable(CodeTeams::Team)))
27+
FilePathTeamCache.set(file_path, T.let(find_team!(T.must(result[:team_name]), allow_raise: allow_raise), T.nilable(CodeTeams::Team)))
2828
end
2929

3030
FilePathTeamCache.get(file_path)
3131
end
3232

33-
sig { params(files: T::Array[String]).returns(T::Hash[String, T.nilable(CodeTeams::Team)]) }
34-
def teams_for_files(files)
33+
sig { params(files: T::Array[String], allow_raise: T::Boolean).returns(T::Hash[String, T.nilable(CodeTeams::Team)]) }
34+
def teams_for_files(files, allow_raise: false)
3535
::RustCodeOwners.teams_for_files(files).each_with_object({}) do |path_team, hash|
3636
file_path, team = path_team
37-
found_team = team ? CodeTeams.find(team[:team_name]) : nil
37+
found_team = team ? find_team!(team[:team_name], allow_raise: allow_raise) : nil
3838
FilePathTeamCache.set(file_path, found_team)
3939
hash[file_path] = found_team
4040
end
@@ -53,7 +53,7 @@ def for_package(package)
5353
owner_name = package.raw_hash['owner'] || package.metadata['owner']
5454
return nil if owner_name.nil?
5555

56-
find_team!(owner_name)
56+
find_team!(owner_name, allow_raise: true)
5757
end
5858

5959
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable(::CodeTeams::Team)) }
@@ -73,10 +73,14 @@ def first_owned_file_for_backtrace(backtrace, excluded_teams: [])
7373
nil
7474
end
7575

76-
sig { params(team_name: String).returns(CodeTeams::Team) }
77-
def find_team!(team_name)
78-
CodeTeams.find(team_name) ||
76+
sig { params(team_name: String, allow_raise: T::Boolean).returns(T.nilable(CodeTeams::Team)) }
77+
def find_team!(team_name, allow_raise: false)
78+
team = CodeTeams.find(team_name)
79+
if team.nil? && allow_raise
7980
raise(StandardError, "Could not find team with name: `#{team_name}`. Make sure the team is one of `#{CodeTeams.all.map(&:name).sort}`")
81+
end
82+
83+
team
8084
end
8185

8286
private_class_method(:find_team!)

‎spec/lib/code_ownership_spec.rb‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,22 @@
6464
expect(subject).to eq({ 'packs/my_pack/owned_file.rb' => nil })
6565
end
6666
end
67+
68+
context 'when ownership is found but team is not found and allow_raise is true' do
69+
let(:files) { ['packs/my_pack/owned_file.rb'] }
70+
before do
71+
allow(RustCodeOwners).to receive(:teams_for_files).and_return({ files.first => { team_name: 'Made Up Team' } })
72+
end
73+
74+
it 'raises an error' do
75+
expect { CodeOwnership.teams_for_files_from_codeowners(files, allow_raise: true) }.to raise_error(StandardError, /Could not find team with name:/)
76+
end
77+
end
6778
end
6879
end
6980

7081
describe '.for_file_from_codeowners' do
71-
subject { CodeOwnership.for_file_from_codeowners(file_path) }
82+
subject { CodeOwnership.for_file(file_path, from_codeowners: true) }
7283

7384
context 'when config is not found' do
7485
let(:file_path) { 'app/javascript/[test]/test.js' }
@@ -124,6 +135,17 @@
124135
expect(subject).to be_nil
125136
end
126137
end
138+
139+
context 'when ownership is found but team is not found and allow_raise is true' do
140+
let(:file_path) { 'packs/my_pack/owned_file.rb' }
141+
before do
142+
allow(RustCodeOwners).to receive(:teams_for_files).and_return({ file_path => { team_name: 'Made Up Team' } })
143+
end
144+
145+
it 'raises an error' do
146+
expect { CodeOwnership.for_file(file_path, from_codeowners: true, allow_raise: true) }.to raise_error(StandardError, /Could not find team with name:/)
147+
end
148+
end
127149
end
128150
end
129151

@@ -139,6 +161,8 @@
139161
context 'with non-empty application' do
140162
before do
141163
create_non_empty_application
164+
# codeowners-rs is matching files against the codeowners file for default path
165+
RustCodeOwners.generate_and_validate(false)
142166
end
143167

144168
context 'when no ownership is found' do
@@ -165,11 +189,25 @@
165189
context 'when ownership is found but team is not found' do
166190
let(:file_path) { 'packs/my_pack/owned_file.rb' }
167191
before do
168-
allow(RustCodeOwners).to receive(:for_file).and_return({ team_name: 'Made Up Team' })
192+
allow(RustCodeOwners).to receive(:teams_for_files).and_return({ file_path => { team_name: 'Made Up Team' } })
169193
end
170194

171-
it 'raises an error' do
172-
expect { subject }.to raise_error(StandardError, /Could not find team with name: `Made Up Team`. Make sure the team is one of/)
195+
it 'returns nil by default' do
196+
expect(subject).to be_nil
197+
end
198+
end
199+
200+
context 'when ownership is found but team is not found and allow_raise is true' do
201+
let(:file_path) { 'packs/my_pack/owned_file.rb' }
202+
203+
it 'raises an error when using from_codeowners path' do
204+
allow(RustCodeOwners).to receive(:teams_for_files).and_return({ file_path => { team_name: 'Made Up Team' } })
205+
expect { CodeOwnership.for_file(file_path, allow_raise: true) }.to raise_error(StandardError, /Could not find team with name:/)
206+
end
207+
208+
it 'raises an error when using single-file path' do
209+
allow(RustCodeOwners).to receive(:for_file).and_return({ team_name: 'Made Up Team' })
210+
expect { CodeOwnership.for_file(file_path, from_codeowners: false, allow_raise: true) }.to raise_error(StandardError, /Could not find team with name:/)
173211
end
174212
end
175213
end

0 commit comments

Comments
 (0)