chore: Improve check pr action (#4937)

* improve check pr action

* Changes from lint:fix

* update log

* Changes from lint:fix

* fix and extend tests

* Changes from lint:fix

* use different word

* Changes from lint:fix

* improve check

* Changes from lint:fix

* fix tests

* Changes from lint:fix

---------

Co-authored-by: cloud-sdk-js <[email protected]>
Co-authored-by: Deeksha Sinha <[email protected]>

build-packages/check-pr/validators.spec.ts

@@ -1,4 +1,5 @@
 import mock from 'mock-fs';
+import * as github from '@actions/core';
 import { validateTitle, validateBody, validateChangesets } from './validators';
 
 const prTemplate = `unchanged PR template
@@ -9,11 +10,13 @@ describe('check-pr', () => {
     mock({
       '.github': {
         'PULL_REQUEST_TEMPLATE.md': prTemplate
-      }
+      },
+      'my-changeset.md': '[Fixed Issue] Something is fixed.'
     });
   });
 
   beforeEach(() => {
+    jest.spyOn(github, 'getInput').mockReturnValue('my-changeset.md');
     process.exitCode = 0;
   });
 
@@ -69,21 +72,50 @@ describe('check-pr', () => {
   });
 
   describe('validatePreamble', () => {
-    it('should invalidate with unmatched changesets', async () => {
+    it('should invalidate with unmatched changesets', () => {
       const fileContents = ["'@sap-cloud-sdk/generator': minor"];
-      await validateChangesets('chore!', '', true, fileContents);
+      validateChangesets('chore!', '', true, fileContents);
       expect(process.exitCode).toEqual(1);
     });
 
+    it('should not fail when preamble is chore and no changeset was created', () => {
+      validateChangesets('chore', '', false, []);
+      expect(process.exitCode).toEqual(0);
+    });
+
+    it('should fail if change type is wrong', async () => {
+      const fileContents = [
+        "'@sap-cloud-sdk/generator': major",
+        '[Fix] Something is fixed.'
+      ];
+      validateChangesets('chore!', '', true, fileContents);
+      expect(process.exitCode).toEqual(1);
+    });
+
+    it('should pass if correct change type is provided', async () => {
+      const fileContents = [
+        "'@sap-cloud-sdk/generator': major",
+        '[Fixed Issue] Something is fixed.'
+      ];
+      validateChangesets('chore!', '', true, fileContents);
+      expect(process.exitCode).toEqual(0);
+    });
+
     it('should validate with matched changesets', async () => {
-      const fileContents = ["'@sap-cloud-sdk/generator': major"];
-      await validateChangesets('chore!', '', true, fileContents);
+      const fileContents = [
+        "'@sap-cloud-sdk/generator': major",
+        '[Fixed Issue] Something is fixed.'
+      ];
+      validateChangesets('chore!', '', true, fileContents);
       expect(process.exitCode).toEqual(0);
     });
 
     it('should validate with matched changesets in double quotes', async () => {
-      const fileContents = ['"@sap-cloud-sdk/generator": major'];
-      await validateChangesets('chore!', '', true, fileContents);
+      const fileContents = [
+        '"@sap-cloud-sdk/generator": major',
+        '[Fixed Issue] Something is fixed.'
+      ];
+      validateChangesets('chore!', '', true, fileContents);
       expect(process.exitCode).toEqual(0);
     });
   });

build-packages/check-pr/validators.ts

@@ -37,7 +37,7 @@ async function validatePreamble(preamble: string): Promise<void> {
     preamble,
     commitType,
     !!isBreaking,
-    await extractChangedFilesContents()
+    await extractChangesetFileContents()
   );
 }
 
@@ -79,10 +79,10 @@ function getAllowedBumps(preamble: string, isBreaking: boolean): string[] {
   return [];
 }
 
-async function hasMatchingChangeset(
+function hasMatchingChangeset(
   allowedBumps: string[],
   changedFileContents: string[]
-): Promise<boolean> {
+): boolean {
   if (allowedBumps.length) {
     return changedFileContents.some(fileContent =>
       allowedBumps.some(bump =>
@@ -94,30 +94,59 @@ async function hasMatchingChangeset(
   return true;
 }
 
-async function extractChangedFilesContents(): Promise<string[]> {
-  const changedFilesStr = getInput('changed-files').trim();
-  const changedFiles = changedFilesStr ? changedFilesStr.split(' ') : [];
+async function extractChangesetFileContents(): Promise<string[]> {
+  const changeSetFilesStr = getInput('changed-files').trim();
+  const changeSetFiles = changeSetFilesStr ? changeSetFilesStr.split(' ') : [];
   const fileContents = await Promise.all(
-    changedFiles.map(file => readFile(file, 'utf-8'))
+    changeSetFiles.map(file => readFile(file, 'utf-8'))
   );
   return fileContents;
 }
 
-export async function validateChangesets(
+export function validateChangesets(
   preamble: string,
   commitType: string,
   isBreaking: boolean,
   fileContents: string[]
-): Promise<void> {
+): void {
   const allowedBumps = getAllowedBumps(commitType, isBreaking);
-  if (!(await hasMatchingChangeset(allowedBumps, fileContents))) {
+  const allowedChangeTypes = [
+    'Known Issue',
+    'Compatibility Note',
+    'New Functionality',
+    'Improvement',
+    'Fixed Issue'
+  ];
+
+  if (!hasMatchingChangeset(allowedBumps, fileContents)) {
     return setFailed(
       `Preamble '${preamble}' requires a changeset file with bump ${allowedBumps
         .map(bump => `'${bump}'`)
         .join(' or ')}.`
     );
   }
 
+  const changeTypes = fileContents.flatMap(content => {
+    const matches = content.match(/\[([^\]]+)\]/g);
+    return matches ? matches.map(match => match.slice(1, -1)) : [];
+  });
+
+  if (preamble !== 'chore' && !changeTypes.length) {
+    return setFailed('Missing change type in changeset.');
+  }
+
+  const allChangeTypesMatch = changeTypes.every(type =>
+    allowedChangeTypes.includes(type)
+  );
+
+  if (!allChangeTypesMatch) {
+    return setFailed(
+      `All change types must match one of the allowed change types ${allowedChangeTypes
+        .map(type => `'[${type}]'`)
+        .join(' or ')}.`
+    );
+  }
+
   info('✓ Changesets: OK');
 }