Merge pull request #4219 from github/mbg/ci/improve-pr-checks-triggers

Improve `pr-checks.yml` triggers and conditions

.github/workflows/__diagnostics-export.yml

@@ -48,15 +48,6 @@ jobs:
     steps:
       - name: Check out repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-      - name: Install Node.js
-        uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
-        with:
-          node-version: 20.x
-          cache: npm
-      - name: Install newer npm
-        run: npm install -g npm@11.19.1
-      - name: Install dependencies
-        run: npm ci
       - name: Prepare test
         id: prepare-test
         uses: ./.github/actions/prepare-test
@@ -90,13 +81,56 @@ jobs:
           name: diagnostics-export-${{ matrix.os }}-${{ matrix.version }}.sarif.json
           path: '${{ runner.temp }}/results/javascript.sarif'
           retention-days: 7
-
       - name: Check diagnostics appear in SARIF
+        uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
         env:
           SARIF_PATH: '${{ runner.temp }}/results/javascript.sarif'
-        run: |
-          cd ../action
-          npx tsx ./pr-checks/check-diagnostics.ts
+        with:
+          script: |
+            const fs = require('fs');
+
+            function checkStatusPageNotification(n) {
+              const expectedMessage = 'Plaintext message';
+              if (n.message.text !== expectedMessage) {
+                core.setFailed(`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`);
+              }
+              if (n.locations.length !== 1) {
+                core.setFailed(`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`);
+              }
+            }
+
+            const sarif = JSON.parse(fs.readFileSync(process.env['SARIF_PATH'], 'utf8'));
+            const run = sarif.runs[0];
+
+            const toolExecutionNotifications = run.invocations[0].toolExecutionNotifications;
+            const statusPageNotifications = toolExecutionNotifications.filter(n =>
+              n.descriptor.id === 'lang/diagnostics/example' && n.properties?.visibility?.statusPage
+            );
+            if (statusPageNotifications.length !== 1) {
+              core.setFailed(
+                'Expected exactly one status page reporting descriptor for this diagnostic in the ' +
+                  `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` +
+                  `${statusPageNotifications.length}. All notification reporting descriptors: ` +
+                  `${JSON.stringify(toolExecutionNotifications)}.`
+              );
+            }
+            checkStatusPageNotification(statusPageNotifications[0]);
+
+            const notifications = run.tool.driver.notifications;
+            const diagnosticNotification = notifications.filter(n =>
+              n.id === 'lang/diagnostics/example' && n.name === 'lang/diagnostics/example' &&
+                n.fullDescription.text === 'Diagnostic name'
+            );
+            if (diagnosticNotification.length !== 1) {
+              core.setFailed(
+                'Expected exactly one notification for this diagnostic in the ' +
+                  `'runs[].tool.driver.notifications[]' SARIF property, but found ` +
+                  `${diagnosticNotification.length}. All notifications: ` +
+                  `${JSON.stringify(notifications)}.`
+              );
+            }
+
+            core.info('Finished diagnostic export test');
     env:
       CODEQL_ACTION_EXPORT_DIAGNOSTICS: true
       CODEQL_ACTION_TEST_MODE: true

.github/workflows/pr-checks.yml

@@ -2,6 +2,9 @@ name: PR Checks
 
 on:
   push:
+    branches:
+      - "main"
+      - "releases/v*"
   pull_request:
   merge_group:
     types: [checks_requested]
@@ -145,12 +148,11 @@ jobs:
 
       - name: Fetch base commit
         id: fetch-base
-        # Forks and Dependabot PRs don't have permission to write comments, so skip the repo size
-        # check in those cases.
+        # Skip this (and subsequent steps) if the PR originates from a fork, since this workflow
+        # won't have the necessary permissions to post comments on the PR.
         if: >-
           github.event_name == 'pull_request' &&
-          github.event.pull_request.head.repo.full_name == github.repository &&
-          github.event.pull_request.user.login != 'dependabot[bot]'
+          github.event.pull_request.head.repo.full_name == github.repository
         env:
           BASE_SHA: ${{ github.event.pull_request.base.sha }}
           HEAD_SHA: ${{ github.event.pull_request.head.sha }}
@@ -209,7 +211,7 @@ jobs:
     if: >-
       github.event_name == 'pull_request' &&
       github.event.pull_request.head.repo.full_name == github.repository &&
-      github.event.pull_request.user.login != 'dependabot[bot]' &&
+      github.actor != 'dependabot[bot]' &&
       needs.other-checks.result == 'success'
     permissions:
       contents: read

pr-checks/check-diagnostics.ts

@@ -1,77 +0,0 @@
-import fs from "fs";
-
-import * as core from "@actions/core";
-
-import * as sarif from "../src/sarif";
-
-function checkStatusPageNotification(n) {
-  const expectedMessage = "Plaintext message";
-  if (n.message.text !== expectedMessage) {
-    core.setFailed(
-      `Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`,
-    );
-  }
-  if (n.locations.length !== 1) {
-    core.setFailed(
-      `Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`,
-    );
-  }
-}
-
-const sarifPath = process.env["SARIF_PATH"];
-
-if (sarifPath === undefined) {
-  throw new Error("The path to a SARIF file must be set in 'SARIF_PATH'.");
-}
-
-const log = JSON.parse(fs.readFileSync(sarifPath, "utf8")) as sarif.Log;
-const run = log.runs[0];
-
-if (run.invocations === undefined) {
-  throw new Error("run.invocations is undefined");
-}
-
-const toolExecutionNotifications =
-  run.invocations[0].toolExecutionNotifications;
-
-if (toolExecutionNotifications === undefined) {
-  throw new Error("toolExecutionNotifications is undefined");
-}
-
-const statusPageNotifications = toolExecutionNotifications.filter(
-  (n) =>
-    n.descriptor?.id === "lang/diagnostics/example" &&
-    n.properties?.visibility?.statusPage,
-);
-if (statusPageNotifications.length !== 1) {
-  core.setFailed(
-    "Expected exactly one status page reporting descriptor for this diagnostic in the " +
-      `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` +
-      `${statusPageNotifications.length}. All notification reporting descriptors: ` +
-      `${JSON.stringify(toolExecutionNotifications)}.`,
-  );
-}
-checkStatusPageNotification(statusPageNotifications[0]);
-
-const notifications = run.tool.driver.notifications;
-
-if (notifications === undefined) {
-  throw new Error("notifications is undefined");
-}
-
-const diagnosticNotification = notifications.filter(
-  (n) =>
-    n.id === "lang/diagnostics/example" &&
-    n.name === "lang/diagnostics/example" &&
-    n.fullDescription?.text === "Diagnostic name",
-);
-if (diagnosticNotification.length !== 1) {
-  core.setFailed(
-    "Expected exactly one notification for this diagnostic in the " +
-      `'runs[].tool.driver.notifications[]' SARIF property, but found ` +
-      `${diagnosticNotification.length}. All notifications: ` +
-      `${JSON.stringify(notifications)}.`,
-  );
-}
-
-core.info("Finished diagnostic export test");

pr-checks/checks/diagnostics-export.yml

@@ -5,7 +5,6 @@ versions:
   - nightly-latest
 env:
   CODEQL_ACTION_EXPORT_DIAGNOSTICS: true
-installNode: true
 steps:
   - uses: ./../action/init
     id: init
@@ -33,10 +32,53 @@ steps:
       name: diagnostics-export-${{ matrix.os }}-${{ matrix.version }}.sarif.json
       path: "${{ runner.temp }}/results/javascript.sarif"
       retention-days: 7
-
   - name: Check diagnostics appear in SARIF
+    uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
     env:
       SARIF_PATH: "${{ runner.temp }}/results/javascript.sarif"
-    run: |
-      cd ../action
-      npx tsx ./pr-checks/check-diagnostics.ts
+    with:
+      script: |
+        const fs = require('fs');
+
+        function checkStatusPageNotification(n) {
+          const expectedMessage = 'Plaintext message';
+          if (n.message.text !== expectedMessage) {
+            core.setFailed(`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`);
+          }
+          if (n.locations.length !== 1) {
+            core.setFailed(`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`);
+          }
+        }
+
+        const sarif = JSON.parse(fs.readFileSync(process.env['SARIF_PATH'], 'utf8'));
+        const run = sarif.runs[0];
+
+        const toolExecutionNotifications = run.invocations[0].toolExecutionNotifications;
+        const statusPageNotifications = toolExecutionNotifications.filter(n =>
+          n.descriptor.id === 'lang/diagnostics/example' && n.properties?.visibility?.statusPage
+        );
+        if (statusPageNotifications.length !== 1) {
+          core.setFailed(
+            'Expected exactly one status page reporting descriptor for this diagnostic in the ' +
+              `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` +
+              `${statusPageNotifications.length}. All notification reporting descriptors: ` +
+              `${JSON.stringify(toolExecutionNotifications)}.`
+          );
+        }
+        checkStatusPageNotification(statusPageNotifications[0]);
+
+        const notifications = run.tool.driver.notifications;
+        const diagnosticNotification = notifications.filter(n =>
+          n.id === 'lang/diagnostics/example' && n.name === 'lang/diagnostics/example' &&
+            n.fullDescription.text === 'Diagnostic name'
+        );
+        if (diagnosticNotification.length !== 1) {
+          core.setFailed(
+            'Expected exactly one notification for this diagnostic in the ' +
+              `'runs[].tool.driver.notifications[]' SARIF property, but found ` +
+              `${diagnosticNotification.length}. All notifications: ` +
+              `${JSON.stringify(notifications)}.`
+          );
+        }
+
+        core.info('Finished diagnostic export test');