diff --git a/.circleci/config.yml b/.circleci/config.yml index e6f6c2bf9af..e323303d24d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -265,7 +265,7 @@ jobs: echo "Skipping changelock check because this is not a PR or is a release branch" exit 0 else - ./scripts/changelog-check -b main -s "${CIRCLE_BRANCH}" + ./scripts/changelog_check.rb -b main -s "${CIRCLE_BRANCH}" fi workflows: diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 5b8c9037404..a901127b448 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -75,7 +75,7 @@ check_changelog: exit 0 else git fetch origin --quiet - ./scripts/changelog-check -b origin/"${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}" -s origin/"${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME}" + ./scripts/changelog_check.rb -b origin/"${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}" -s origin/"${CI_MERGE_REQUEST_SOURCE_BRANCH_NAME}" fi install: extends: .on_push diff --git a/scripts/changelog-check b/scripts/changelog_check.rb similarity index 86% rename from scripts/changelog-check rename to scripts/changelog_check.rb index b598f61886b..703392a2f81 100755 --- a/scripts/changelog-check +++ b/scripts/changelog_check.rb @@ -23,6 +23,16 @@ def build_changelog_from_commit(commit) build_changelog(commit.title) end +def get_git_log(base_branch, source_branch) + format = '--pretty=title: %s%nbody:%b%nDELIMITER' + log, status = Open3.capture2( + 'git', 'log', format, "#{base_branch}..#{source_branch}" + ) + + raise 'git log failed' unless status.success? + log +end + # Transforms a formatted git log into structured objects. # The git format ends up printing a single commit as: # @@ -32,15 +42,8 @@ def build_changelog_from_commit(commit) # DELIMITER # The string is first split on DELIMITER, and then the body is split into # individual lines. -def get_git_log(base_branch, source_branch) - format = '--pretty=title: %s%nbody:%b%nDELIMITER' - log, status = Open3.capture2( - 'git', 'log', format, "#{base_branch}..#{source_branch}" - ) - - raise 'git log failed' unless status.success? - - log.strip.split('DELIMITER').map { |commit| +def build_structured_git_log(git_log) + git_log.strip.split('DELIMITER').map { |commit| commit.split("\nbody:").map do |commit_message_lines| commit_message_lines.split(%r{[\r\n]}).filter { |line| line != '' } end @@ -63,6 +66,15 @@ def commit_messages_contain_skip_changelog?(base_branch, source_branch) log.include?(SKIP_CHANGELOG_MESSAGE) end +def generate_invalid_changes(git_log) + log = build_structured_git_log(git_log) + log.reject do |commit| + commit.title.include?(SKIP_CHANGELOG_MESSAGE) || + commit.commit_messages.any? { |message| message.include?(SKIP_CHANGELOG_MESSAGE) } || + build_changelog_from_commit(commit) + end.map(&:title) +end + # Get the last valid changelog line for every Pull Request and tie it to the commit subject. # Each PR should be squashed, which results in every PR being one commit. The commit messages # in a squashed PR are concatencated with a leading "*" for each commit. Example: @@ -78,12 +90,13 @@ def commit_messages_contain_skip_changelog?(base_branch, source_branch) # changelog: Authentication: Updating Authentication (LG-9998) # # * Authentication commit #2 -def generate_changelog(base_branch, source_branch) - log = get_git_log(base_branch, source_branch) +def generate_changelog(git_log) + log = build_structured_git_log(git_log) changelog_entries = [] log.each do |item| # Skip this commit if the skip changelog message appears + next if item.title.include?(SKIP_CHANGELOG_MESSAGE) next if item.commit_messages.any? { |message| message.include?(SKIP_CHANGELOG_MESSAGE) } change = build_changelog_from_commit(item) next unless change @@ -99,7 +112,7 @@ def generate_changelog(base_branch, source_branch) category: category, subcategory: change[:subcategory].capitalize, pr_number: pr_number&.named_captures&.fetch('pr'), - change: change[:change].capitalize, + change: change[:change].sub(/./, &:upcase), ) changelog_entries << changelog_entry @@ -169,7 +182,10 @@ def main(args) abort(optparse.help) if options[:source_branch].nil? - changelog_entries = generate_changelog(options[:base_branch], options[:source_branch]) + git_log = get_git_log(options[:base_branch], options[:source_branch]) + changelog_entries = generate_changelog(git_log) + invalid_changelog_entries = generate_invalid_changes(git_log) + skip_check = commit_messages_contain_skip_changelog?( options[:base_branch], options[:source_branch], @@ -178,6 +194,11 @@ def main(args) if skip_check || changelog_entries.count > 0 formatted_changelog = format_changelog(changelog_entries) puts format_changelog(changelog_entries) if formatted_changelog.length > 0 + if invalid_changelog_entries.count > 0 + puts "\n!!! Invalid Changelog Entries !!!" + puts invalid_changelog_entries.join("\n") + end + exit 0 else warn( diff --git a/spec/fixtures/git_log_changelog.yml b/spec/fixtures/git_log_changelog.yml new file mode 100644 index 00000000000..045ac5d74a2 --- /dev/null +++ b/spec/fixtures/git_log_changelog.yml @@ -0,0 +1,69 @@ +squashed_commit_with_one_commit: + commit_log: | + title: Upgrade Rails to 6.1.4.7 (#6041) + body:changelog: Internal, Security, Upgrade Rails to patch vulnerability + DELIMITER + title: 'Upgrade Rails to 6.1.4.7 (#6041)' + commit_messages: + - 'changelog: Internal, Security, Upgrade Rails to patch vulnerability' + category: 'Internal' + subcategory: 'Security' + change: 'Upgrade Rails to patch vulnerability' + pr_number: '6041' +squashed_commit_with_duplicate_pr: + commit_log: | + title: Upgrade Rails to 6.1.4.8 (#6042) + body:changelog: Internal, Security, Upgrade Rails to patch vulnerability + DELIMITER + title: 'Upgrade Rails to 6.1.4.8 (#6042)' + commit_messages: + - 'changelog: Internal, Security, Upgrade Rails to patch vulnerability' + category: 'Internal' + subcategory: 'Security' + change: 'Upgrade Rails to patch vulnerability' + pr_number: '6042' +squashed_commit_with_multiple_commits: + commit_log: | + title: LG-5515: update platform auth (#5976) + body:* add check to see if platform is available + + * add alert + + * changelog: Improvements, Webauthn, Provide better error flow for users who may not be able to leverage webauthn (LG-5515) + + * fix linting, debug + Co-authored-by: Jessica Dembe + Co-authored-by: Zach Margolis + DELIMITER + title: 'LG-5515: update platform auth (#5976)' + commit_messages: + - '* add check to see if platform is available' + - '* add alert' + - '* changelog: Improvements, Webauthn, Provide better error flow for users who may not be able to leverage webauthn (LG-5515)' + - '* fix linting, debug' + - 'Co-authored-by: Jessica Dembe ' + - 'Co-authored-by: Zach Margolis ' + category: 'Improvements' + subcategory: 'Webauthn' + change: 'Provide better error flow for users who may not be able to leverage webauthn (LG-5515)' + pr_number: '5976' +squashed_commit_with_skip: + commit_log: | + title: Add LOGIN_TASK_LOG_LEVEL env var (#6037) + body:- Lets us set log level to minimize STDOUT output + from Identity::Hostdata (downloading files from S3, etc) + + [skip changelog] + DELIMITER + title: 'Add LOGIN_TASK_LOG_LEVEL env var (#6037)' + commit_messages: + - '- Lets us set log level to minimize STDOUT output' + - ' from Identity::Hostdata (downloading files from S3, etc)' + - '[skip changelog]' +squashed_commit_invalid: + commit_log: | + title: LG-5629 account buttons part 2 (#5945) + body: + DELIMITER + title: 'LG-5629 account buttons part 2 (#5945)' + commit_messages: [] diff --git a/spec/scripts/changelog_check_spec.rb b/spec/scripts/changelog_check_spec.rb new file mode 100644 index 00000000000..b2fedb60cf7 --- /dev/null +++ b/spec/scripts/changelog_check_spec.rb @@ -0,0 +1,106 @@ +require 'rails_helper' +require_relative '../../scripts/changelog_check' + +RSpec.describe 'scripts/changelog_check' do + git_fixtures = YAML.safe_load(File.read(File.expand_path('spec/fixtures/git_log_changelog.yml'))) + + describe '#build_changelog' do + it 'builds a git log into structured changelog objects' do + git_log = git_fixtures.values.pluck('commit_log').join("\n") + changelog_entries = generate_changelog(git_log) + expect(changelog_entries.length).to eq 3 + fixture_and_changelog = git_fixtures.values.filter do |x| + x['category'].present? + end.zip(changelog_entries) + + fixture_and_changelog.each do |fixture, changelog| + expect(fixture['category']).to eq changelog.category + expect(fixture['subcategory']).to eq changelog.subcategory + expect(fixture['pr_number']).to eq changelog.pr_number + expect(fixture['change']).to eq changelog.change + end + end + + it 'skips commits with [skip changelog]' do + commits = [ + git_fixtures['squashed_commit_with_one_commit'], + git_fixtures['squashed_commit_with_skip'], + ] + git_log = commits.pluck('commit_log').join("\n") + changelog = generate_changelog(git_log) + expect(changelog.length).to eq 1 + + expect(commits.first['category']).to eq changelog.first.category + expect(commits.first['subcategory']).to eq changelog.first.subcategory + expect(commits.first['pr_number']).to eq changelog.first.pr_number + expect(commits.first['change']).to eq changelog.first.change + end + end + + describe '#build_structured_git_log' do + it 'builds a git log into structured objects' do + git_fixtures.values.each do |commit_fixture| + commits = build_structured_git_log(commit_fixture['commit_log']) + expect(commits.length).to eq 1 + expect(commits.first.title).to eq commit_fixture['title'] + expect(commits.first.commit_messages).to eq commit_fixture['commit_messages'] + end + end + end + + describe '#generate_invalid_changes' do + it 'returns changelog entries without a valid structure' do + commits = [ + git_fixtures['squashed_commit_invalid'], + git_fixtures['squashed_commit_with_one_commit'], + git_fixtures['squashed_commit_with_skip'], + ] + git_log = commits.pluck('commit_log').join("\n") + changes = generate_invalid_changes(git_log) + + expect(changes.length).to eq 1 + expect(commits.first['title']).to eq changes.first + end + end + + describe '#format_changelog' do + it 'returns changelog entries without a valid structure' do + commits = [ + git_fixtures['squashed_commit_invalid'], + git_fixtures['squashed_commit_with_one_commit'], + git_fixtures['squashed_commit_with_skip'], + git_fixtures['squashed_commit_with_duplicate_pr'], + git_fixtures['squashed_commit_with_multiple_commits'], + ] + git_log = commits.pluck('commit_log').join("\n") + changelogs = generate_changelog(git_log) + formatted_changelog = format_changelog(changelogs) + + expect(formatted_changelog).to eq <<~CHANGELOG.chomp + ## Improvements + - Webauthn: Provide better error flow for users who may not be able to leverage webauthn (LG-5515) ([#5976](https://github.com/18F/identity-idp/pull/5976)) + + ## Internal + - Security: Upgrade Rails to patch vulnerability ([#6041](https://github.com/18F/identity-idp/pull/6041), [#6042](https://github.com/18F/identity-idp/pull/6042)) + CHANGELOG + end + end + + describe '#generate_changelog' do + it 'capitalizes subcategory and capitalizes first letter of change description' do + git_log = <<~COMMIT + title: Add LOGIN_TASK_LOG_LEVEL env var (#6037) + body:- Lets us set log level to minimize STDOUT output + from Identity::Hostdata (downloading files from S3, etc) + + * changelog: Improvements, authentication, provide better authentication (LG-4515) + DELIMITER + COMMIT + + changelogs = generate_changelog(git_log) + + expect(changelogs.first.subcategory).to eq('Authentication') + expect(changelogs.first.change).to start_with('P') + end + end +end