[summarize-checks] Only set status=success if requirementsMet=true (#36609)

- add a new test case for completed FYI, incomplete required, no assessment
- we shouldn't acccidentally set to SUCCESS when dealing with only completed FYI, when we don't yet know the target branch

Author: scbedd

.github/workflows/src/summarize-checks/summarize-checks.js

@@ -1011,7 +1011,7 @@ function getCommentBody(
 
     if (anyFyiPresent) {
       bodyProper += getFyiPresentBody(failingFyiChecksInfo);
-      if (!anyBlockerPresent) {
+      if (!anyBlockerPresent && requirementsMet) {
         bodyProper += `If you still want to proceed merging this PR without addressing the above failures, ${diagramTsg(4, false)}.`;
         summaryData =
           `⚠️ Some important automated merging requirements have failed. As of today you can still merge this PR, ` +

.github/workflows/test/summarize-checks/summarize-checks.test.js

@@ -704,6 +704,65 @@ describe("Summarize Checks Unit Tests", () => {
       expect(output).toEqual(expectedOutput);
     });
 
+    it("should generate pending summary when checks there are no required checks blocking or completed, but successful FYI checks", async () => {
+      const repo = "azure-rest-api-specs";
+      const targetBranch = "main";
+      const labelNames = [
+        "ARMReview",
+        "ARMAutoSignedOff",
+        "resource-manager",
+        "TypeSpec",
+        "RPaaS",
+        "ARMSignedOff",
+        "PublishToCustomers",
+      ];
+      const expectedOutput = [
+        `<h2>Next Steps to Merge</h2>Important checks have failed. As of today they are not blocking this PR, but in near future they may.<br/>Addressing the following failures is highly recommended:<br/><ul><li>⚠️ The check named <code>Swagger LintDiff</code> has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the <a href="https://aka.ms/ci-fix">aka.ms/ci-fix</a> guide</li></ul><br /><br />Comment generated by <a href="http://github.com/a/fake/workflowrun/url">summarize-checks</a> workflow run.`,
+        {
+          name: "[TEST-IGNORE] Automated merging requirements met",
+          result: "pending",
+          summary: "The requirements for merging this PR are still being evaluated. Please wait.",
+        },
+      ];
+
+      const fyiCheckRuns = [
+        {
+          name: "Swagger BreakingChange",
+          status: "COMPLETED",
+          conclusion: "SUCCESS",
+          checkInfo: getCheckInfo("Swagger BreakingChange"),
+        },
+        {
+          name: "Swagger LintDiff",
+          status: "COMPLETED",
+          conclusion: "FAILURE",
+          checkInfo: getCheckInfo("Swagger LintDiff"),
+        },
+      ];
+
+      const requiredCheckRuns = [
+        {
+          name: "Summarize PR Impact",
+          status: "IN_PROGRESS",
+          conclusion: null,
+          checkInfo: getCheckInfo("Summarize PR Impact"),
+        },
+      ];
+
+      const output = await createNextStepsComment(
+        mockCore,
+        repo,
+        labelNames,
+        targetBranch,
+        requiredCheckRuns,
+        fyiCheckRuns,
+        false, // assessmentCompleted
+        WORKFLOW_URL,
+      );
+
+      expect(output).toEqual(expectedOutput);
+    });
+
     it("Should generate error summary for a PR scenario with labeling issues", async () => {
       const repo = "azure-rest-api-specs";
       const targetBranch = "main";