Skip to content

Commit 2d877bc

Browse files
perryqhperryqh
authored
Handling file paths with square brackets (#28)
* adding failing test for special char in path * escaping brackets in globs * codeowner entries need brackets escaped too
1 parent 8d27cdd commit 2d877bc

File tree

5 files changed

+268
-142
lines changed

5 files changed

+268
-142
lines changed

‎README.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@
22
This is a CLI tool to generate [GitHub `CODEOWNERS` files](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) from an existing project assuming certain conventions around file annotations and Ruby/Javascript packages.
33
It's also the [oxidation](https://wiki.mozilla.org/Oxidation) of an existing [CLI tool](https://github.com/rubyatscale/code_ownership) written in Ruby.
44

5-
`CODEOWNERS` generation happens as part of our git commit hooks and on Gusto's main repo takes ~18s to run. The Rust implementation which is a drop in replacement cuts that down to <= 2s. (Tested on a Mackbook M1)
6-
75
### Documentation
6+
87
```bash
98
A CLI to validate and generate Github's CODEOWNERS file
109

‎src/common_test.rs‎

Lines changed: 146 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ pub mod tests {
1010

1111
use crate::{ownership::Ownership, project::Project};
1212

13+
macro_rules! ownership {
14+
($($test_files:expr),+) => {{
15+
let temp_dir = tempdir()?;
16+
let test_config = TestConfig::new(
17+
temp_dir.path().to_path_buf(),
18+
vec![$($test_files),+]
19+
);
20+
build_ownership(test_config)
21+
}};
22+
}
23+
1324
const DEFAULT_CODE_OWNERSHIP_YML: &str = "
1425
---
1526
owned_globs:
@@ -108,95 +119,112 @@ team_file_glob:
108119
}
109120

110121
pub fn build_ownership_with_directory_codeowners() -> Result<Ownership, Box<dyn Error>> {
111-
let temp_dir = tempdir()?;
122+
ownership!(
123+
TestProjectFile {
124+
relative_path: "app/consumers/deep/nesting/nestdir/deep_file.rb".to_owned(),
125+
content: "class DeepFile\nend\n".to_owned(),
126+
},
127+
TestProjectFile {
128+
relative_path: "app/consumers/one_owner.rb".to_owned(),
129+
content: "class OneOwner\nend\n".to_owned(),
130+
},
131+
TestProjectFile {
132+
relative_path: "app/services/service_file.rb".to_owned(),
133+
content: "class ServiceFile\nend\n".to_owned(),
134+
},
135+
TestProjectFile {
136+
relative_path: "app/services/some_other_file.rb".to_owned(),
137+
content: "class SomeOtherFile\nend\n".to_owned(),
138+
},
139+
TestProjectFile {
140+
relative_path: "app/consumers/.codeowner".to_owned(),
141+
content: "Bar\n".to_owned(),
142+
},
143+
TestProjectFile {
144+
relative_path: "app/services/.codeowner".to_owned(),
145+
content: "Foo\n".to_owned(),
146+
},
147+
TestProjectFile {
148+
relative_path: "app/services/exciting/.codeowner".to_owned(),
149+
content: "Bar\n".to_owned(),
150+
}
151+
)
152+
}
112153

113-
let test_config = TestConfig::new(
114-
temp_dir.path().to_path_buf(),
115-
vec![
116-
TestProjectFile {
117-
relative_path: "app/consumers/deep/nesting/nestdir/deep_file.rb".to_owned(),
118-
content: "class DeepFile\nend\n".to_owned(),
119-
},
120-
TestProjectFile {
121-
relative_path: "app/consumers/one_owner.rb".to_owned(),
122-
content: "class OneOwner\nend\n".to_owned(),
123-
},
124-
TestProjectFile {
125-
relative_path: "app/services/service_file.rb".to_owned(),
126-
content: "class ServiceFile\nend\n".to_owned(),
127-
},
128-
TestProjectFile {
129-
relative_path: "app/services/some_other_file.rb".to_owned(),
130-
content: "class SomeOtherFile\nend\n".to_owned(),
131-
},
132-
TestProjectFile {
133-
relative_path: "app/consumers/.codeowner".to_owned(),
134-
content: "Bar\n".to_owned(),
135-
},
136-
TestProjectFile {
137-
relative_path: "app/services/.codeowner".to_owned(),
138-
content: "Foo\n".to_owned(),
139-
},
140-
TestProjectFile {
141-
relative_path: "app/services/exciting/.codeowner".to_owned(),
142-
content: "Bar\n".to_owned(),
143-
},
144-
],
145-
);
146-
Ok(build_ownership(test_config)?)
154+
pub fn build_ownership_with_directory_codeowners_with_brackets() -> Result<Ownership, Box<dyn Error>> {
155+
ownership!(
156+
TestProjectFile {
157+
relative_path: "app/[consumers]/deep/nesting/[nestdir]/deep_file.rb".to_owned(),
158+
content: "class DeepFile\nend\n".to_owned(),
159+
},
160+
TestProjectFile {
161+
relative_path: "app/[consumers]/one_owner.rb".to_owned(),
162+
content: "class OneOwner\nend\n".to_owned(),
163+
},
164+
TestProjectFile {
165+
relative_path: "app/services/service_file.rb".to_owned(),
166+
content: "class ServiceFile\nend\n".to_owned(),
167+
},
168+
TestProjectFile {
169+
relative_path: "app/services/some_other_file.rb".to_owned(),
170+
content: "class SomeOtherFile\nend\n".to_owned(),
171+
},
172+
TestProjectFile {
173+
relative_path: "app/[consumers]/.codeowner".to_owned(),
174+
content: "Bar\n".to_owned(),
175+
},
176+
TestProjectFile {
177+
relative_path: "app/[consumers]/deep/nesting/[nestdir]/.codeowner".to_owned(),
178+
content: "Foo\n".to_owned(),
179+
}
180+
)
147181
}
148182

149183
pub fn build_ownership_with_all_mappers() -> Result<Ownership, Box<dyn Error>> {
150-
let temp_dir = tempdir()?;
151-
152-
let test_config = TestConfig::new(
153-
temp_dir.path().to_path_buf(),
154-
vec![
155-
TestProjectFile {
156-
relative_path: "app/consumers/directory_owned.rb".to_owned(),
157-
content: "class DirectoryOwned\nend\n".to_owned(),
158-
},
159-
TestProjectFile {
160-
relative_path: "app/consumers/.codeowner".to_owned(),
161-
content: "Bar\n".to_owned(),
162-
},
163-
TestProjectFile {
164-
relative_path: "packs/foo/package.yml".to_owned(),
165-
content: "owner: Baz\n".to_owned(),
166-
},
167-
TestProjectFile {
168-
relative_path: "packs/foo/app/services/package_owned.rb".to_owned(),
169-
content: "class PackageOwned\nend\n".to_owned(),
170-
},
171-
TestProjectFile {
172-
relative_path: "packs/bar/app/services/team_file_owned.rb".to_owned(),
173-
content: "class GlobMapperOwned\nend\n".to_owned(),
174-
},
175-
TestProjectFile {
176-
relative_path: "config/teams/baz.yml".to_owned(),
177-
content: "name: Baz\ngithub:\n team: \"@Baz\"\n members:\n - Baz member\nowned_globs:\n - \"packs/bar/**\"\n"
178-
.to_owned(),
179-
},
180-
TestProjectFile {
181-
relative_path: "packs/zebra/app/services/team_file_owned.rb".to_owned(),
182-
content: "# @team Foo\nclass TeamFileOwned\nend\n".to_owned(),
183-
},
184-
TestProjectFile {
185-
relative_path: "packs/jscomponents/comp.ts".to_owned(),
186-
content: "// @team Foo\n".to_owned(),
187-
},
188-
TestProjectFile {
189-
relative_path: "gems/taco/sauce.rb".to_owned(),
190-
content: "class Taco::Sauce\nend\n".to_owned(),
191-
},
192-
TestProjectFile {
193-
relative_path: "config/teams/bam.yml".to_owned(),
194-
content: "name: Bam\ngithub:\n team: \"@Bam\"\n members:\n - Bam member\nruby:\n owned_gems:\n - taco\n"
195-
.to_owned(),
196-
},
197-
],
198-
);
199-
Ok(build_ownership(test_config)?)
184+
ownership!(
185+
TestProjectFile {
186+
relative_path: "app/consumers/directory_owned.rb".to_owned(),
187+
content: "class DirectoryOwned\nend\n".to_owned(),
188+
},
189+
TestProjectFile {
190+
relative_path: "app/consumers/.codeowner".to_owned(),
191+
content: "Bar\n".to_owned(),
192+
},
193+
TestProjectFile {
194+
relative_path: "packs/foo/package.yml".to_owned(),
195+
content: "owner: Baz\n".to_owned(),
196+
},
197+
TestProjectFile {
198+
relative_path: "packs/foo/app/services/package_owned.rb".to_owned(),
199+
content: "class PackageOwned\nend\n".to_owned(),
200+
},
201+
TestProjectFile {
202+
relative_path: "packs/bar/app/services/team_file_owned.rb".to_owned(),
203+
content: "class GlobMapperOwned\nend\n".to_owned(),
204+
},
205+
TestProjectFile {
206+
relative_path: "config/teams/baz.yml".to_owned(),
207+
content: "name: Baz\ngithub:\n team: \"@Baz\"\n members:\n - Baz member\nowned_globs:\n - \"packs/bar/**\"\n"
208+
.to_owned(),
209+
},
210+
TestProjectFile {
211+
relative_path: "packs/zebra/app/services/team_file_owned.rb".to_owned(),
212+
content: "# @team Foo\nclass TeamFileOwned\nend\n".to_owned(),
213+
},
214+
TestProjectFile {
215+
relative_path: "packs/jscomponents/comp.ts".to_owned(),
216+
content: "// @team Foo\n".to_owned(),
217+
},
218+
TestProjectFile {
219+
relative_path: "gems/taco/sauce.rb".to_owned(),
220+
content: "class Taco::Sauce\nend\n".to_owned(),
221+
},
222+
TestProjectFile {
223+
relative_path: "config/teams/bam.yml".to_owned(),
224+
content: "name: Bam\ngithub:\n team: \"@Bam\"\n members:\n - Bam member\nruby:\n owned_gems:\n - taco\n"
225+
.to_owned(),
226+
}
227+
)
200228
}
201229
pub fn build_ownership_with_team_file_codeowners() -> Result<Ownership, Box<dyn Error>> {
202230
let temp_dir = tempdir()?;
@@ -212,37 +240,24 @@ team_file_glob:
212240
Ok(build_ownership(test_config)?)
213241
}
214242
pub fn build_ownership_with_team_gem_codeowners() -> Result<Ownership, Box<dyn Error>> {
215-
let temp_dir = tempdir()?;
216-
217-
let test_config = TestConfig::new(
218-
temp_dir.path().to_path_buf(),
219-
vec![
220-
TestProjectFile {
221-
relative_path: "gems/globbing/globber.rb".to_owned(),
222-
content: "class Globbing::Globber\nend\n".to_owned(),
223-
},
224-
TestProjectFile {
225-
relative_path: "config/teams/bam.yml".to_owned(),
226-
content: "name: Bam\ngithub:\n team: \"@Bam\"\n members:\n - Bam member\nruby:\n owned_gems:\n - globbing\n"
227-
.to_owned(),
228-
},
229-
],
230-
);
231-
Ok(build_ownership(test_config)?)
243+
ownership!(
244+
TestProjectFile {
245+
relative_path: "gems/globbing/globber.rb".to_owned(),
246+
content: "class Globbing::Globber\nend\n".to_owned(),
247+
},
248+
TestProjectFile {
249+
relative_path: "config/teams/bam.yml".to_owned(),
250+
content: "name: Bam\ngithub:\n team: \"@Bam\"\n members:\n - Bam member\nruby:\n owned_gems:\n - globbing\n"
251+
.to_owned(),
252+
}
253+
)
232254
}
233255

234256
pub fn build_ownership_with_team_glob_codeowners() -> Result<Ownership, Box<dyn Error>> {
235-
let temp_dir = tempdir()?;
236-
237-
let test_config = TestConfig::new(
238-
temp_dir.path().to_path_buf(),
239-
vec![TestProjectFile {
240-
relative_path: "config/teams/baz.yml".to_owned(),
241-
content: "name: Baz\ngithub:\n team: \"@Baz\"\n members:\n - Baz member\nowned_globs:\n - \"packs/bar/**\"\n"
242-
.to_owned(),
243-
}],
244-
);
245-
Ok(build_ownership(test_config)?)
257+
ownership!(TestProjectFile {
258+
relative_path: "config/teams/baz.yml".to_owned(),
259+
content: "name: Baz\ngithub:\n team: \"@Baz\"\n members:\n - Baz member\nowned_globs:\n - \"packs/bar/**\"\n".to_owned(),
260+
})
246261
}
247262

248263
pub fn build_ownership_with_team_yml_codeowners() -> Result<Ownership, Box<dyn Error>> {
@@ -252,29 +267,23 @@ team_file_glob:
252267
Ok(build_ownership(test_config)?)
253268
}
254269
pub fn build_ownership_with_package_codeowners() -> Result<Ownership, Box<dyn Error>> {
255-
let temp_dir = tempdir()?;
256-
257-
let test_config = TestConfig::new(
258-
temp_dir.path().to_path_buf(),
259-
vec![
260-
TestProjectFile {
261-
relative_path: "packs/foo/package.yml".to_owned(),
262-
content: "owner: Baz\n".to_owned(),
263-
},
264-
TestProjectFile {
265-
relative_path: "packs/foo/app/services/package_owned.rb".to_owned(),
266-
content: "class PackageOwned\nend\n".to_owned(),
267-
},
268-
TestProjectFile {
269-
relative_path: "packs/bam/package.yml".to_owned(),
270-
content: "owner: Bam\n".to_owned(),
271-
},
272-
TestProjectFile {
273-
relative_path: "packs/bam/app/services/package_owned.rb".to_owned(),
274-
content: "class PackageOwned\nend\n".to_owned(),
275-
},
276-
],
277-
);
278-
Ok(build_ownership(test_config)?)
270+
ownership!(
271+
TestProjectFile {
272+
relative_path: "packs/foo/package.yml".to_owned(),
273+
content: "owner: Baz\n".to_owned(),
274+
},
275+
TestProjectFile {
276+
relative_path: "packs/foo/app/services/package_owned.rb".to_owned(),
277+
content: "class PackageOwned\nend\n".to_owned(),
278+
},
279+
TestProjectFile {
280+
relative_path: "packs/bam/package.yml".to_owned(),
281+
content: "owner: Bam\n".to_owned(),
282+
},
283+
TestProjectFile {
284+
relative_path: "packs/bam/app/services/package_owned.rb".to_owned(),
285+
content: "class PackageOwned\nend\n".to_owned(),
286+
}
287+
)
279288
}
280289
}

‎src/ownership/mapper.rs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,47 @@ mod tests {
143143
Some((&team_name_longest, &source_longest))
144144
);
145145
}
146+
147+
fn assert_owner_for(glob: &str, relative_path: &str, expect_match: bool) {
148+
let source = "directory_mapper (\"packs/bam\")".to_string();
149+
let team_name = "team1".to_string();
150+
let owner_matcher = OwnerMatcher::Glob {
151+
glob: glob.to_string(),
152+
team_name: team_name.clone(),
153+
source: source.clone(),
154+
};
155+
let response = owner_matcher.owner_for(&Path::new(relative_path));
156+
if expect_match {
157+
assert_eq!(response, (Some(&team_name), &source));
158+
} else {
159+
assert_eq!(response, (None, &source));
160+
}
161+
}
162+
163+
#[test]
164+
fn owner_for_without_brackets_in_glob() {
165+
assert_owner_for("packs/bam/**/**", "packs/bam/app/components/sidebar.jsx", true);
166+
assert_owner_for("packs/bam/**/**", "packs/baz/app/components/sidebar.jsx", false);
167+
assert_owner_for("packs/bam/**/**", "packs/bam/app/[components]/gadgets/sidebar.jsx", true);
168+
assert_owner_for("packs/bam/**/**", "packs/bam/app/sidebar_[component].jsx", true);
169+
}
170+
171+
#[test]
172+
fn owner_for_with_brackets_in_glob() {
173+
assert_owner_for(
174+
"packs/bam/app/\\[components\\]/**/**",
175+
"packs/bam/app/[components]/gadgets/sidebar.jsx",
176+
true,
177+
);
178+
assert_owner_for("packs/\\[bam\\]/**/**", "packs/[bam]/app/components/sidebar.jsx", true);
179+
}
180+
181+
#[test]
182+
fn owner_for_with_multiple_brackets_in_glob() {
183+
assert_owner_for(
184+
"packs/\\[bam\\]/bar/\\[foo\\]/**/**",
185+
"packs/[bam]/bar/[foo]/app/components/sidebar.jsx",
186+
true,
187+
);
188+
}
146189
}

0 commit comments

Comments
 (0)