71519 (#17958)

Rerun flaky tests automatically, and create issue reports for them (#71519)

Adds a new workflow that will try to automatically detect and rerun
flaky tests, and create an issue report for them.

The detection mechanism is heuristic: if exactly ONE job fails in the CI
Suite, then it is assumed to be flaky, and will be rerun. If the next
run succeeds, then it will create an issue report for that flaky test if
one does not already exist. It will do its best to create a unique but
consistent identifier, aided by PRs like #71515. You can find an example
here: https://github.com/Mothblocks/ss13-workflow-testing/issues/20.
Maintainers can also rename the issue if they wish, it will still be
able to find it.

While there is a chance for this mechanism to go wrong and create bogus
issue reports, it IS possible to easily disable actions, I did it for
the stale one just a bit ago. Most likely, this mechanism going wrong is
going to be the result of randomness leaking in tests, like random human
names, so this can be solved in the tests themselves. I find it
extremely unlikely, but in the worst case scenario where this happens
often, we can add a way for maintainers to edit the issue report and
include a regex to match for runtimes. Just an idea.

Includes a few large-ish downloaded logs from past failures that are
interesting in unique ways. These are used for tests of the title
generator.

Co-authored-by: Mothblocks <35135081+Mothblocks@users.noreply.github.com>

.github/workflows/rerun_flaky_tests.yml

@@ -0,0 +1,31 @@
+name: Rerun/Report Flaky Tests
+on:
+  workflow_run:
+    workflows: [CI Suite]
+    types:
+    - completed
+jobs:
+  rerun_flaky_tests:
+    runs-on: ubuntu-latest
+    if: ${{ github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.run_attempt == 1 }}
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+    - name: Rerun flaky tests
+      uses: actions/github-script@v6
+      with:
+        script: |
+          const { rerunFlakyTests } = await import('${{ github.workspace }}/tools/pull_request_hooks/rerunFlakyTests.js')
+          await rerunFlakyTests({ github, context })
+  report_flaky_tests:
+    runs-on: ubuntu-latest
+    if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.run_attempt == 2 }}
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+    - name: Report flaky tests
+      uses: actions/github-script@v6
+      with:
+        script: |
+          const { reportFlakyTests } = await import('${{ github.workspace }}/tools/pull_request_hooks/rerunFlakyTests.js')
+          await reportFlakyTests({ github, context })

tools/pull_request_hooks/rerunFlakyTests.js

@@ -0,0 +1,275 @@
+const LABEL = "🤖 Flaky Test Report";
+const TITLE_BOT_HEADER = "title: ";
+
+// Only check jobs that start with these.
+// Helps make sure we don't restart something like screenshot tests or linters, which are not known to be flaky.
+const CONSIDERED_JOBS = [
+  "CI Suite / Integration Tests",
+];
+
+async function getFailedJobsForRun(github, context, workflowRunId, runAttempt) {
+  const {
+    data: { jobs },
+  } = await github.rest.actions.listJobsForWorkflowRunAttempt({
+    owner: context.repo.owner,
+    repo: context.repo.repo,
+    run_id: workflowRunId,
+    attempt_number: runAttempt,
+  });
+
+  return jobs
+    .filter((job) => job.conclusion === "failure")
+    .filter((job) =>
+      CONSIDERED_JOBS.some((title) => job.name.startsWith(title))
+    );
+}
+
+export async function rerunFlakyTests({ github, context }) {
+  const failingJobs = await getFailedJobsForRun(
+    github,
+    context,
+    context.payload.workflow_run.id,
+    context.payload.workflow_run.run_attempt
+  );
+
+  if (failingJobs.length > 1) {
+    console.log("Multiple jobs failing. PROBABLY not flaky, not rerunning.");
+    return;
+  }
+
+  if (failingJobs.length === 0) {
+    throw new Error(
+      "rerunFlakyTests should not have run on a run with no failing jobs"
+    );
+  }
+
+  github.rest.actions.reRunWorkflowFailedJobs({
+    owner: context.repo.owner,
+    repo: context.repo.repo,
+    run_id: context.payload.workflow_run.id,
+  });
+}
+
+// Tries its best to extract a useful error title and message for the given log
+export function extractDetails(log) {
+  // Strip off timestamp
+  const lines = log.split(/^[0-9.:T\-]*?Z /gm);
+
+  const failureRegex = /^\t?FAILURE #(?<number>[0-9]+): (?<headline>.+)/;
+  const groupRegex = /^##\[group\](?<group>.+)/;
+
+  const failures = [];
+  let lastGroup = "root";
+  let loggingFailure;
+
+  const newFailure = (failureMatch) => {
+    const { headline } = failureMatch.groups;
+
+    loggingFailure = {
+      headline,
+      group: lastGroup.replace("/datum/unit_test/", ""),
+      details: [],
+    };
+  };
+
+  for (const line of lines) {
+    const groupMatch = line.match(groupRegex);
+    if (groupMatch) {
+      lastGroup = groupMatch.groups.group.trim();
+      continue;
+    }
+
+    const failureMatch = line.match(failureRegex);
+
+    if (loggingFailure === undefined) {
+      if (!failureMatch) {
+        continue;
+      }
+
+      newFailure(failureMatch);
+    } else if (failureMatch || line.startsWith("##")) {
+      failures.push(loggingFailure);
+      loggingFailure = undefined;
+
+      if (failureMatch) {
+        newFailure(failureMatch);
+      }
+    } else {
+      loggingFailure.details.push(line.trim());
+    }
+  }
+
+  // We had no logged failures, there's not really anything we can do here
+  if (failures.length === 0) {
+    return {
+      title: "Flaky test failure with no obvious source",
+      failures,
+    };
+  }
+
+  // We *could* create multiple failures for multiple groups.
+  // This would be important if we had multiple flaky tests at the same time.
+  // I'm choosing not to because it complicates this logic a bit, has the ability to go terribly wrong,
+  // and also because there's something funny to me about that increasing the urgency of fixing
+  // flaky tests. If it becomes a serious issue though, I would not mind this being fixed.
+  const uniqueGroups = new Set(failures.map((failure) => failure.group));
+
+  if (uniqueGroups.size > 1) {
+    return {
+      title: `Multiple flaky test failures in ${Array.from(uniqueGroups)
+        .sort()
+        .join(", ")}`,
+      failures,
+    };
+  }
+
+  const failGroup = failures[0].group;
+
+  if (failures.length > 1) {
+    return {
+      title: `Multiple errors in flaky test ${failGroup}`,
+      failures,
+    };
+  }
+
+  const failure = failures[0];
+
+  // Common patterns where we can always get a detailed title
+  const runtimeMatch = failure.headline.match(/Runtime in .+?: (?<error>.+)/);
+  if (runtimeMatch) {
+    return {
+      title: `Flaky test ${failGroup}: ${runtimeMatch.groups.error.trim()}`,
+      failures,
+    };
+  }
+
+  // Try to normalize the title and remove anything that might be variable
+  const normalizedError = failure.headline.replace(/\s*at .+?:[0-9]+.*/g, ""); // "<message> at code.dm:123"
+
+  return {
+    title: `Flaky test ${failGroup}: ${normalizedError}`,
+    failures,
+  };
+}
+
+async function getExistingIssueId(graphql, context, title) {
+  // Hope you never have more than 100 of these open!
+  const {
+    repository: {
+      issues: { nodes: openFlakyTestIssues },
+    },
+  } = await graphql(
+    `
+      query ($owner: String!, $repo: String!, $label: String!) {
+        repository(owner: $owner, name: $repo) {
+          issues(
+            labels: [$label]
+            first: 100
+            orderBy: { field: CREATED_AT, direction: DESC }
+            states: [OPEN]
+          ) {
+            nodes {
+              number
+              title
+              body
+            }
+          }
+        }
+      }
+    `,
+    {
+      owner: context.repo.owner,
+      repo: context.repo.repo,
+      label: LABEL,
+    }
+  );
+
+  const exactTitle = openFlakyTestIssues.find((issue) => issue.title === title);
+  if (exactTitle !== undefined) {
+    return exactTitle.number;
+  }
+
+  const foundInBody = openFlakyTestIssues.find((issue) =>
+    issue.body.contains(`<!-- ${TITLE_BOT_HEADER}${exactTitle} -->`)
+  );
+  if (foundInBody !== undefined) {
+    return foundInBody.number;
+  }
+
+  return undefined;
+}
+
+function createBody({ title, failures }, runUrl) {
+  return `
+	<!-- This issue can be renamed, but do not change the next comment! -->
+	<!-- title: ${title} -->
+
+	Flaky tests were detected in [this test run](${runUrl}). This means that there was a failure that was cleared when the tests were simply restarted.
+	
+	Failures:
+	\`\`\`
+	${failures
+    .map(
+      (failure) =>
+        `${failure.group}: ${failure.headline}\n\t${failure.details.join("\n")}`
+    )
+    .join("\n")}
+	\`\`\`
+	`.replace(/^\s*/gm, "");
+}
+
+export async function reportFlakyTests({ github, context }) {
+  const failedJobsFromLastRun = await getFailedJobsForRun(
+    github,
+    context,
+    context.payload.workflow_run.id,
+    context.payload.workflow_run.run_attempt - 1
+  );
+
+  // This could one day be relaxed if we face serious enough flaky test problems, so we're going to loop anyway
+  if (failedJobsFromLastRun.length !== 1) {
+    console.log(
+      "Multiple jobs failing after retry, assuming maintainer rerun."
+    );
+
+    return;
+  }
+
+  for (const job of failedJobsFromLastRun) {
+    const { data: log } =
+      await github.rest.actions.downloadJobLogsForWorkflowRun({
+        owner: context.repo.owner,
+        repo: context.repo.repo,
+        job_id: job.id,
+      });
+
+    const details = extractDetails(log);
+
+    const existingIssueId = await getExistingIssueId(
+      github.graphql,
+      context,
+      details.title
+    );
+
+    if (existingIssueId !== undefined) {
+      // Maybe in the future, if it's helpful, update the existing issue with new links
+      console.log(`Existing issue found: #${existingIssueId}`);
+      return;
+    }
+
+    await github.rest.issues.create({
+      owner: context.repo.owner,
+      repo: context.repo.repo,
+      title: details.title,
+      labels: [LABEL],
+      body: createBody(
+        details,
+        `https://github.com/${context.repo.owner}/${
+          context.repo.repo
+        }/actions/runs/${context.payload.workflow_run.id}/attempts/${
+          context.payload.workflow_run.run_attempt - 1
+        }`
+      ),
+    });
+  }
+}

tools/pull_request_hooks/rerunFlakyTests.test.js

@@ -0,0 +1,39 @@
+import { strict as assert } from "node:assert";
+import fs from "node:fs";
+import { extractDetails } from "./rerunFlakyTests.js";
+
+function extractDetailsFromPayload(filename) {
+  return extractDetails(
+    fs.readFileSync(`tests/flakyTestPayloads/${filename}.txt`, {
+      encoding: "utf8",
+    })
+  );
+}
+
+const chatClient = extractDetailsFromPayload("chat_client");
+assert.equal(
+  chatClient.title,
+  "Flaky test create_and_destroy: /datum/computer_file/program/chatclient hard deleted 1 times out of a total del count of 13"
+);
+assert.equal(chatClient.failures.length, 1);
+
+const monkeyBusiness = extractDetailsFromPayload("monkey_business");
+assert.equal(
+  monkeyBusiness.title,
+  "Flaky test monkey_business: Cannot execute null.resolve()."
+);
+assert.equal(monkeyBusiness.failures.length, 1);
+
+const shapeshift = extractDetailsFromPayload("shapeshift");
+assert.equal(
+  shapeshift.title,
+  "Multiple errors in flaky test shapeshift_spell"
+);
+assert.equal(shapeshift.failures.length, 16);
+
+const multipleFailures = extractDetailsFromPayload("multiple_failures");
+assert.equal(
+  multipleFailures.title,
+  "Multiple flaky test failures in more_shapeshift_spell, shapeshift_spell"
+);
+assert.equal(multipleFailures.failures.length, 2);

tools/pull_request_hooks/tests/flakyTestPayloads/multiple_failures.txt

@@ -0,0 +1,8 @@
+2022-11-22T05:59:45.2618397Z ##[group]/datum/unit_test/shapeshift_spell
+2022-11-22T05:59:45.4118582Z ##[error]Shapeshift spell: Wild Shapeshift failed to transform the dummy into the shape Juggernaut. (Pablo Pfeifer was located within the floor, which is a /turf/open/floor/iron).
+2022-11-22T05:59:45.4119786Z 	FAILURE #1: Shapeshift spell: Dragon Form failed to transform the dummy into the shape . (Pablo Pfeifer was located within the floor, which is a /turf/open/floor/iron). at code/modules/unit_tests/spell_shapeshift.dm:65
+2022-11-22T05:59:45.2618397Z ##[endgroup]
+2022-11-22T05:59:45.2618397Z ##[group]/datum/unit_test/more_shapeshift_spell
+2022-11-22T05:59:45.4118582Z ##[error]Shapeshift spell: Wild Shapeshift failed to transform the dummy into the shape Juggernaut. (Pablo Pfeifer was located within the floor, which is a /turf/open/floor/iron).
+2022-11-22T05:59:45.4119786Z 	FAILURE #1: Shapeshift spell: Dragon Form failed to transform the dummy into the shape . (Pablo Pfeifer was located within the floor, which is a /turf/open/floor/iron). at code/modules/unit_tests/spell_shapeshift.dm:65
+2022-11-22T05:59:45.2618397Z ##[endgroup]