From 351ae33e34b8f160d020004548291e1549a72a66 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sat, 2 Mar 2024 18:30:11 -0500 Subject: [PATCH] update workflow with improved comment structure Signed-off-by: aramissennyeydd Signed-off-by: web-next-automation --- .../api-breaking-changes-comment.yml | 12 +- .github/workflows/api-breaking-changes.yml | 2 - .../src/commands/repo/schema/openapi/check.ts | 62 ++++++++-- .../src/lib/openapi/optic/helpers.ts | 112 +++++++++++++++--- .../src/schema/openapi.generated.ts | 2 +- 5 files changed, 157 insertions(+), 33 deletions(-) diff --git a/.github/workflows/api-breaking-changes-comment.yml b/.github/workflows/api-breaking-changes-comment.yml index 91741bba5b..4c7fa452e7 100644 --- a/.github/workflows/api-breaking-changes-comment.yml +++ b/.github/workflows/api-breaking-changes-comment.yml @@ -77,7 +77,7 @@ jobs: run: | unzip preview-spec.zip comment.md ls - echo "MANIFESTS_FILE_HASH=$(md5sum manifests.rendered.yml | awk '{ print $1 }')" >> $GITHUB_OUTPUT + grep add-comment: name: Write comment about issues @@ -93,19 +93,19 @@ jobs: uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4 # Identify comment to be updated - - name: Find comment for Ephemeral Environment + - name: Find comment for API Changes uses: peter-evans/find-comment@d5fe37641ad8451bdd80312415672ba26c86575e # v3 id: find-comment with: - issue-number: ${{ needs.cache-manifests-file.outputs.pr-number }} + issue-number: ${{ needs.setup.outputs.pr-number }} comment-author: 'github-actions[bot]' - body-includes: pr-changes-${{ needs.cache-manifests-file.outputs.pr-number }} + body-includes: API changes direction: last - - name: Create or Update Comment with Deployment URL + - name: Create or Update Comment with API Changes uses: peter-evans/create-or-update-comment@71345be0265236311c031f5c7866368bd1eff043 # v4 with: - comment-id: ${{ steps.notification.outputs.comment-id }} + comment-id: ${{ steps.find-comment.outputs.comment-id }} issue-number: ${{ github.event.pull_request.number }} body-path: comment.md edit-mode: replace diff --git a/.github/workflows/api-breaking-changes.yml b/.github/workflows/api-breaking-changes.yml index 5c75910546..07206718bb 100644 --- a/.github/workflows/api-breaking-changes.yml +++ b/.github/workflows/api-breaking-changes.yml @@ -12,8 +12,6 @@ jobs: name: Build PR image runs-on: ubuntu-latest if: ${{ github.event_name != 'pull_request' || github.event.action != 'closed' }} - outputs: - tags: ${{ steps.meta.outputs.tags }} steps: - name: Harden Runner uses: step-security/harden-runner@63c24ba6bd7ba022e95695ff85de572c04a18142 # v2.7.0 diff --git a/packages/repo-tools/src/commands/repo/schema/openapi/check.ts b/packages/repo-tools/src/commands/repo/schema/openapi/check.ts index 5599937322..d434e0cd4d 100644 --- a/packages/repo-tools/src/commands/repo/schema/openapi/check.ts +++ b/packages/repo-tools/src/commands/repo/schema/openapi/check.ts @@ -20,6 +20,8 @@ import { CiRunDetails, generateCompareSummaryMarkdown, } from '../../../../lib/openapi/optic/helpers'; +import { paths as cliPaths } from '../../../../lib/paths'; +import { YAML_SCHEMA_PATH } from '../../../../lib/openapi/constants'; export async function command(opts: OptionValues) { let packages = await PackageGraph.listTargetPackages(); @@ -28,26 +30,34 @@ export async function command(opts: OptionValues) { if (opts.since) { const { stdout: sinceRaw } = await exec('git', ['rev-parse', opts.since]); since = sinceRaw.toString().trim(); - const graph = PackageGraph.fromPackages(packages); - const changedPackages = await graph.listChangedPackages({ - ref: opts.since, - analyzeLockfile: true, - }); - const withDevDependents = graph.collectPackageNames( - changedPackages.map(pkg => pkg.name), - pkg => pkg.localDevDependents.keys(), + const { stdout: changedFilesRaw } = await exec('git', [ + 'diff', + '--name-only', + since, + ]); + const changedFiles = changedFilesRaw.toString().trim(); + + const changedOpenApiSpecs = changedFiles + .split('\n') + .filter(e => e.endsWith(YAML_SCHEMA_PATH)) + .map(e => cliPaths.resolveTarget(e)); + + // filter packages by changedFiles + packages = packages.filter(pkg => + changedOpenApiSpecs.some(e => e.startsWith(`${pkg.dir}/`)), ); - packages = Array.from(withDevDependents).map(name => graph.get(name)!); } const checkablePackages = packages.filter( e => e.packageJson.scripts?.['check:api'], ); + try { const outputs = { completed: [], failed: [], noop: [], + warning: [], severity: 0, } as CiRunDetails; for (const pkg of checkablePackages) { @@ -65,6 +75,40 @@ export async function command(opts: OptionValues) { outputs.noop.push(...(result.noop ?? [])); } + for (const pkg of packages.filter( + e => !e.packageJson.scripts?.['check:api'], + )) { + outputs.warning?.push({ + apiName: `${pkg.dir}/`, + warning: 'No check:api script found in package.json', + }); + } + + outputs.completed.forEach( + e => + (e.apiName = e.apiName + .replace(cliPaths.targetDir, '') + .replace(YAML_SCHEMA_PATH, '')), + ); + outputs.failed.forEach( + e => + (e.apiName = e.apiName + .replace(cliPaths.targetDir, '') + .replace(YAML_SCHEMA_PATH, '')), + ); + outputs.noop.forEach( + e => + (e.apiName = e.apiName + .replace(cliPaths.targetDir, '') + .replace(YAML_SCHEMA_PATH, '')), + ); + outputs.warning?.forEach( + e => + (e.apiName = e.apiName + .replace(cliPaths.targetDir, '') + .replace(YAML_SCHEMA_PATH, '')), + ); + const { stdout: currentSha } = await exec('git', ['rev-parse', 'HEAD']); console.log( generateCompareSummaryMarkdown( diff --git a/packages/repo-tools/src/lib/openapi/optic/helpers.ts b/packages/repo-tools/src/lib/openapi/optic/helpers.ts index 9bcd8c45a3..2393e71ce8 100644 --- a/packages/repo-tools/src/lib/openapi/optic/helpers.ts +++ b/packages/repo-tools/src/lib/openapi/optic/helpers.ts @@ -23,8 +23,6 @@ import { getOperationsChanged, } from '@useoptic/openapi-utilities'; import { GroupedDiffs } from '@useoptic/openapi-utilities/build/openapi3/group-diff'; -import { relative } from 'path'; -import { paths as cliPaths } from '../../paths'; /** * The below code is copied from https://github.com/opticdev/optic/blob/main/projects/optic/src/commands/ci/comment/common.ts#L82 for use @@ -45,6 +43,7 @@ export type CiRunDetails = { specUrl?: string | null; capture?: any; }[]; + warning?: { apiName: string; warning: string }[]; failed: { apiName: string; error: string }[]; noop: { apiName: string }[]; severity: Severity; @@ -121,6 +120,18 @@ const getCaptureIssuesLabel = ({ ].join('\n'); }; +const getBreakagesRow = (breakage: CiRunDetails['completed'][number]) => { + return ` + - ${breakage.apiName} + ${breakage.comparison.results.map( + s => ` + - ${s.where} + ${'```'} + ${s.error} + ${'```'}`, + )}`; +}; + export const generateCompareSummaryMarkdown = ( commit: { sha: string }, results: CiRunDetails, @@ -130,10 +141,61 @@ export const generateCompareSummaryMarkdown = ( s => s.warnings.length > 0, ); const anyCompletedHasCapture = results.completed.some(s => s.capture); - return ` + if ( + results.completed.length === 0 && + results.failed.length === 0 && + results.failed.length === 0 + ) { + return `No API changes detected for commit (${commit.sha})`; + } + const breakages = results.completed + .filter(s => s.comparison.results.some(e => !e.passed)) + .map(e => ({ + ...e, + comparison: { + ...e.comparison, + results: e.comparison.results.filter(f => !f.passed), + }, + })); + const successfullyCompletedCount = + results.completed.length - breakages.length; + return `### Summary for commit (${commit.sha}) + +${ + results.noop.length > 0 + ? `${ + results.noop.length === 1 ? '1 API' : `${results.noop.length} APIs` + } had no changes.` + : '' +} +${ + breakages.length > 0 + ? `${ + breakages.length === 1 ? '1 API' : `${breakages.length} APIs` + } had breaking changes.` + : '' +} +${ + successfullyCompletedCount > 0 + ? `${ + successfullyCompletedCount === 1 + ? '1 API' + : `${successfullyCompletedCount} APIs` + } had non-breaking changes.` + : '' +} +${ + results.warning && results.warning.length > 0 + ? `${ + results.warning.length === 1 + ? '1 API' + : `${results.warning.length} APIs` + } had warnings.` + : '' +} ${ results.completed.length > 0 - ? `### APIs Changed + ? `### APIs with Changes @@ -151,7 +213,7 @@ ${results.completed s => `
-${relative(cliPaths.targetDir, s.apiName)} +${s.apiName} ${getOperationsText(s.comparison.groupedDiffs, { @@ -190,13 +252,13 @@ ${ ) .join('\n')} -
-` +` : '' } + ${ results.failed.length > 0 - ? `### Errors running optic + ? `### APIs with Errors @@ -226,13 +288,33 @@ ${'```'} : '' } -Summary of API changes for commit (${commit.sha}) - ${ - results.noop.length > 0 - ? `${ - results.noop.length === 1 ? '1 API' : `${results.noop.length} APIs` - } had no changes.` + results.warning && results.warning.length + ? ` +### APIs with Warnings +
+ + + + + + + + ${results.warning + .map(e => ``) + .join('\n')} + +
APIWarning
${e.apiName}${e.warning}
` : '' -}`; +} +${ + breakages.length > 0 + ? ` +### Routes with Breakages + +${breakages.map(getBreakagesRow).join('\n')} +` + : '' +} +`; }; diff --git a/plugins/catalog-backend/src/schema/openapi.generated.ts b/plugins/catalog-backend/src/schema/openapi.generated.ts index c8c9c73a84..550db58f2e 100644 --- a/plugins/catalog-backend/src/schema/openapi.generated.ts +++ b/plugins/catalog-backend/src/schema/openapi.generated.ts @@ -1072,7 +1072,7 @@ export const spec = { 'application/json': { schema: { type: 'object', - required: ['entityRefs', 'fields'], + required: ['entityRefs'], properties: { entityRefs: { type: 'array',