Show any invalid entries in changelog

.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:

.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

scripts/changelog-check → 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(

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 <jessica.dembe@gsa.gov>
+    Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
+    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 <jessica.dembe@gsa.gov>'
+    - 'Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>'
+  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: []

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