Skip to content

Commit 485173f

Browse files
author
Alex Evanczuk
authored
Improve validation message when CODEOWNERS is out of date (#9)
* Show diff when CODEOWNERS is out of date * bump version * add extra test cases * Better format the validation message * remove mention of pre-commit hook -n * remove accidentally added files
1 parent 2f3f1fa commit 485173f

File tree

4 files changed

+205
-12
lines changed

4 files changed

+205
-12
lines changed

‎Gemfile.lock‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: .
33
specs:
4-
code_ownership (1.26.0)
4+
code_ownership (1.27.0)
55
bigrails-teams
66
parse_packwerk
77
sorbet-runtime

‎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.26.0'
3+
spec.version = '1.27.0'
44
spec.authors = ['Gusto Engineers']
55
spec.email = ['[email protected]']
66
spec.summary = 'A gem to help engineering teams declare ownership of code'

‎lib/code_ownership/private/validations/github_codeowners_up_to_date.rb‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,66 @@ def validation_errors(files:, autocorrect: true, stage_changes: true)
2525
# https://help.github.com/en/articles/about-code-owners
2626
HEADER
2727

28-
contents = [
29-
header,
28+
expected_content_lines = [
29+
*header.split("\n"),
30+
nil, # For line between header and codeowners_file_lines
3031
*codeowners_file_lines,
3132
nil, # For end-of-file newline
32-
].join("\n")
33+
]
34+
35+
expected_contents = expected_content_lines.join("\n")
36+
actual_contents = codeowners_filepath.exist? ? codeowners_filepath.read : ""
37+
actual_content_lines = actual_contents.split("\n")
3338

34-
codeowners_up_to_date = codeowners_filepath.exist? && codeowners_filepath.read == contents
39+
codeowners_up_to_date = actual_contents == expected_contents
3540

3641
errors = T.let([], T::Array[String])
3742

3843
if !codeowners_up_to_date
3944
if autocorrect
40-
codeowners_filepath.write(contents)
45+
codeowners_filepath.write(expected_contents)
4146
if stage_changes
4247
`git add #{codeowners_filepath}`
4348
end
4449
else
45-
errors << "CODEOWNERS out of date. Ensure pre-commit hook is set up correctly and used. You can also run bin/codeownership validate to update the CODEOWNERS file\n"
50+
# If there is no current file or its empty, display a shorter message.
51+
missing_lines = expected_content_lines - actual_content_lines
52+
extra_lines = actual_content_lines - expected_content_lines
53+
missing_lines_text = if missing_lines.any?
54+
<<~COMMENT
55+
CODEOWNERS should contain the following lines, but does not:
56+
#{(expected_content_lines - actual_content_lines).map { |line| "- \"#{line}\""}.join("\n")}
57+
COMMENT
58+
end
59+
60+
extra_lines_text = if extra_lines.any?
61+
<<~COMMENT
62+
CODEOWNERS should not contain the following lines, but it does:
63+
#{(actual_content_lines - expected_content_lines).map { |line| "- \"#{line}\""}.join("\n")}
64+
COMMENT
65+
end
66+
67+
diff_text = if missing_lines_text && extra_lines_text
68+
"#{missing_lines_text}\n#{extra_lines_text}".chomp
69+
elsif missing_lines_text
70+
missing_lines_text
71+
elsif extra_lines_text
72+
extra_lines_text
73+
else
74+
""
75+
end
76+
77+
if actual_contents == ""
78+
errors << <<~CODEOWNERS_ERROR
79+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
80+
CODEOWNERS_ERROR
81+
else
82+
errors << <<~CODEOWNERS_ERROR
83+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
84+
85+
#{diff_text.chomp}
86+
CODEOWNERS_ERROR
87+
end
4688
end
4789
end
4890

‎spec/lib/code_ownership_spec.rb‎

Lines changed: 155 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@
313313
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
314314
puts e.message
315315
expect(e.message).to eq <<~EXPECTED.chomp
316-
CODEOWNERS out of date. Ensure pre-commit hook is set up correctly and used. You can also run bin/codeownership validate to update the CODEOWNERS file
316+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
317317
318318
See https://github.com/bigrails/code_ownership#README.md for more details
319319
EXPECTED
@@ -332,7 +332,7 @@
332332
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
333333
puts e.message
334334
expect(e.message).to eq <<~EXPECTED.chomp
335-
CODEOWNERS out of date. Ensure pre-commit hook is set up correctly and used. You can also run bin/codeownership validate to update the CODEOWNERS file
335+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
336336
337337
See https://github.com/bigrails/code_ownership#README.md for more details
338338
EXPECTED
@@ -357,7 +357,7 @@
357357
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
358358
puts e.message
359359
expect(e.message).to eq <<~EXPECTED.chomp
360-
CODEOWNERS out of date. Ensure pre-commit hook is set up correctly and used. You can also run bin/codeownership validate to update the CODEOWNERS file
360+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
361361
362362
See https://github.com/bigrails/code_ownership#README.md for more details
363363
EXPECTED
@@ -386,7 +386,7 @@
386386
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
387387
puts e.message
388388
expect(e.message).to eq <<~EXPECTED.chomp
389-
CODEOWNERS out of date. Ensure pre-commit hook is set up correctly and used. You can also run bin/codeownership validate to update the CODEOWNERS file
389+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
390390
391391
See https://github.com/bigrails/code_ownership#README.md for more details
392392
EXPECTED
@@ -395,6 +395,157 @@
395395
end
396396
end
397397
end
398+
399+
context 'in an application with a CODEOWNERS file that is missing lines and has extra lines' do
400+
before { create_non_empty_application }
401+
402+
it 'prints out the diff' do
403+
FileUtils.mkdir('.github')
404+
Pathname.new('.github/CODEOWNERS').write <<~CODEOWNERS
405+
# STOP! - DO NOT EDIT THIS FILE MANUALLY
406+
# This file was automatically generated by "bin/codeownership validate".
407+
#
408+
# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
409+
# teams. This is useful when developers create Pull Requests since the
410+
# code/file owner is notified. Reference GitHub docs for more details:
411+
# https://help.github.com/en/articles/about-code-owners
412+
413+
414+
# Annotations at the top of file
415+
/frontend/javascripts/packages/my_package/owned_file.jsx @MyOrg/bar-team
416+
/frontend/some/extra/line/that/should/not/exist @MyOrg/bar-team
417+
418+
# Team-specific owned globs
419+
/app/services/bar_stuff/** @MyOrg/bar-team
420+
/frontend/javascripts/bar_stuff/** @MyOrg/bar-team
421+
422+
# Some extra comment that should not be here
423+
424+
# Owner metadata key in package.json
425+
/frontend/javascripts/packages/my_other_package/**/** @MyOrg/bar-team
426+
CODEOWNERS
427+
428+
expect_any_instance_of(codeowners_validation).to_not receive(:`) # rubocop:disable RSpec/AnyInstance
429+
expect { CodeOwnership.validate!(autocorrect: false) }.to raise_error do |e|
430+
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
431+
puts e.message
432+
expect(e.message).to eq <<~EXPECTED.chomp
433+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
434+
435+
CODEOWNERS should contain the following lines, but does not:
436+
- ""
437+
- "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
438+
- "# Owner metadata key in package.yml"
439+
- "/packs/my_other_package/**/** @MyOrg/bar-team"
440+
- ""
441+
442+
CODEOWNERS should not contain the following lines, but it does:
443+
- "/frontend/some/extra/line/that/should/not/exist @MyOrg/bar-team"
444+
- "# Some extra comment that should not be here"
445+
446+
See https://github.com/bigrails/code_ownership#README.md for more details
447+
EXPECTED
448+
end
449+
end
450+
end
451+
452+
context 'in an application with a CODEOWNERS file that has extra lines' do
453+
before { create_non_empty_application }
454+
455+
it 'prints out the diff' do
456+
FileUtils.mkdir('.github')
457+
Pathname.new('.github/CODEOWNERS').write <<~CODEOWNERS
458+
# STOP! - DO NOT EDIT THIS FILE MANUALLY
459+
# This file was automatically generated by "bin/codeownership validate".
460+
#
461+
# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
462+
# teams. This is useful when developers create Pull Requests since the
463+
# code/file owner is notified. Reference GitHub docs for more details:
464+
# https://help.github.com/en/articles/about-code-owners
465+
466+
467+
# Annotations at the top of file
468+
/frontend/javascripts/packages/my_package/owned_file.jsx @MyOrg/bar-team
469+
/packs/my_pack/owned_file.rb @MyOrg/bar-team
470+
/frontend/some/extra/line/that/should/not/exist @MyOrg/bar-team
471+
472+
# Team-specific owned globs
473+
/app/services/bar_stuff/** @MyOrg/bar-team
474+
/frontend/javascripts/bar_stuff/** @MyOrg/bar-team
475+
476+
# Owner metadata key in package.yml
477+
/packs/my_other_package/**/** @MyOrg/bar-team
478+
479+
# Some extra comment that should not be here
480+
481+
# Owner metadata key in package.json
482+
/frontend/javascripts/packages/my_other_package/**/** @MyOrg/bar-team
483+
484+
CODEOWNERS
485+
486+
expect_any_instance_of(codeowners_validation).to_not receive(:`) # rubocop:disable RSpec/AnyInstance
487+
expect { CodeOwnership.validate!(autocorrect: false) }.to raise_error do |e|
488+
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
489+
puts e.message
490+
expect(e.message).to eq <<~EXPECTED.chomp
491+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
492+
493+
CODEOWNERS should not contain the following lines, but it does:
494+
- "/frontend/some/extra/line/that/should/not/exist @MyOrg/bar-team"
495+
- "# Some extra comment that should not be here"
496+
497+
See https://github.com/bigrails/code_ownership#README.md for more details
498+
EXPECTED
499+
end
500+
end
501+
end
502+
503+
context 'in an application with a CODEOWNERS file that has missing lines' do
504+
before { create_non_empty_application }
505+
506+
it 'prints out the diff' do
507+
FileUtils.mkdir('.github')
508+
Pathname.new('.github/CODEOWNERS').write <<~CODEOWNERS
509+
# STOP! - DO NOT EDIT THIS FILE MANUALLY
510+
# This file was automatically generated by "bin/codeownership validate".
511+
#
512+
# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
513+
# teams. This is useful when developers create Pull Requests since the
514+
# code/file owner is notified. Reference GitHub docs for more details:
515+
# https://help.github.com/en/articles/about-code-owners
516+
517+
518+
# Annotations at the top of file
519+
/frontend/javascripts/packages/my_package/owned_file.jsx @MyOrg/bar-team
520+
521+
# Team-specific owned globs
522+
/app/services/bar_stuff/** @MyOrg/bar-team
523+
/frontend/javascripts/bar_stuff/** @MyOrg/bar-team
524+
525+
# Owner metadata key in package.json
526+
/frontend/javascripts/packages/my_other_package/**/** @MyOrg/bar-team
527+
528+
CODEOWNERS
529+
530+
expect_any_instance_of(codeowners_validation).to_not receive(:`) # rubocop:disable RSpec/AnyInstance
531+
expect { CodeOwnership.validate!(autocorrect: false) }.to raise_error do |e|
532+
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
533+
puts e.message
534+
expect(e.message).to eq <<~EXPECTED.chomp
535+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
536+
537+
CODEOWNERS should contain the following lines, but does not:
538+
- ""
539+
- "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
540+
- "# Owner metadata key in package.yml"
541+
- "/packs/my_other_package/**/** @MyOrg/bar-team"
542+
- ""
543+
544+
See https://github.com/bigrails/code_ownership#README.md for more details
545+
EXPECTED
546+
end
547+
end
548+
end
398549
end
399550

400551
context 'code_ownership.yml has skip_codeowners_validation set' do

0 commit comments

Comments
 (0)