Merge pull request #98 from openconfig/dont-omit-output-when-script-f…

…ails Dont omit output when script fails
openconfig · Feb 5, 2024 · c1701f1 · c1701f1
2 parents f533fdd + adc850d
commit c1701f1
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 21 deletions.
diff --git a/post_results/main.go b/post_results/main.go
@@ -336,34 +336,50 @@ func getResult(validatorId, resultsDir string, condensed bool) (string, bool, ve
 	// versionRecords contains all information regarding YANG version changes.
 	var versionRecords versionRecordSlice
 
-	failFileBytes, err := ioutil.ReadFile(filepath.Join(resultsDir, commonci.FailFileName))
+	failFileBytes, err := os.ReadFile(filepath.Join(resultsDir, commonci.FailFileName))
 	// existent fail file == failure.
 	executionFailed := err == nil
 	err = nil
 
 	switch {
-	case executionFailed:
-		outString = string(failFileBytes)
-		if outString == "" {
-			outString = "Test failed with no stderr output."
-		}
-		// For per-model validators, an execution failure suggests a CI infra failure.
-		if validator.IsPerModel {
-			outString = fmt.Sprintf("Validator script failed -- infra bug?\n%s", blockQuote(outString))
-		}
-		pass = false
 	case validator.IsPerModel && validatorId == "misc-checks":
 		outString, pass, versionRecords, err = processMiscChecksOutput(resultsDir)
 	case validator.IsPerModel:
 		outString, pass, err = parseModelResultsHTML(validatorId, resultsDir, condensed)
 		if pass && condensed {
-			outString = "All passed.\n" + outString
+			outString = "All models passed.\n" + outString
 		}
-	default:
+	case !executionFailed:
 		outString = "Test passed."
 		pass = true
 	}
 
+	if executionFailed {
+		failString := string(failFileBytes)
+		if failString == "" {
+			failString = "Test failed with no stderr output."
+		}
+		// For per-model validators, an execution failure suggests a CI infra failure.
+		if validator.IsPerModel {
+			failString = fmt.Sprintf("Validator script failed -- infra bug?\n%s", blockQuote(failString))
+		}
+		pass = false
+
+		// The processing error could be due to the validator script
+		// failing, so catch it and don't fail the result posting
+		// process.
+		if err != nil {
+			failString += "\n" + fmt.Sprint(err)
+			err = nil
+		}
+
+		if outString != "" {
+			outString = failString + "\n" + outString
+		} else {
+			outString = failString
+		}
+	}
+
 	return outString, pass, versionRecords, err
 }
 
@@ -406,7 +422,7 @@ func getGistHeading(validatorId, version, resultsDir string) (string, string, er
 	validatorDesc := validator.StatusName(version)
 	// If version is latest, then get the concrete version output by the tool if it exists.
 	if version == "" {
-		if outBytes, err := ioutil.ReadFile(filepath.Join(resultsDir, commonci.LatestVersionFileName)); err != nil {
+		if outBytes, err := os.ReadFile(filepath.Join(resultsDir, commonci.LatestVersionFileName)); err != nil {
 			log.Printf("INFO: did not read latest version for %s: %v", validatorId, err)
 		} else {
 			// Get the first line of the version output as the tool's display title.
@@ -419,7 +435,7 @@ func getGistHeading(validatorId, version, resultsDir string) (string, string, er
 		}
 	}
 
-	outBytes, err := ioutil.ReadFile(filepath.Join(resultsDir, commonci.OutFileName))
+	outBytes, err := os.ReadFile(filepath.Join(resultsDir, commonci.OutFileName))
 	if err != nil {
 		return "", "", err
 	}

diff --git a/post_results/main_test.go b/post_results/main_test.go
@@ -309,7 +309,7 @@ Passed.
 </details>
 </details>
 `,
-		wantCondensedOut: `All passed.
+		wantCondensedOut: `All models passed.
 `,
 	}, {
 		name:                 "pyang with an empty fail file",
@@ -319,8 +319,35 @@ Passed.
 		wantOut: `Validator script failed -- infra bug?
 ` + "```" + `
 Test failed with no stderr output.
-` + "```",
-		wantCondensedOutSame: true,
+` + "```" + `
+<details>
+  <summary>&#x2705;&nbsp; acl</summary>
+<details>
+  <summary>&#x2705;&nbsp; openconfig-acl</summary>
+Passed.
+</details>
+</details>
+<details>
+  <summary>&#x2705;&nbsp; optical-transport</summary>
+<details>
+  <summary>&#x2705;&nbsp; openconfig-optical-amplifier</summary>
+Passed.
+</details>
+<details>
+  <summary>&#x2705;&nbsp; openconfig-transport-line-protection</summary>
+Passed.
+<ul>
+  <pre>warning foo</pre>
+</ul>
+</details>
+</details>
+`,
+		wantCondensedOut: `Validator script failed -- infra bug?
+` + "```" + `
+Test failed with no stderr output.
+` + "```" + `
+All models passed.
+`,
 	}, {
 		name:                 "basic non-pyang pass",
 		inValidatorResultDir: "testdata/oc-pyang",
@@ -352,7 +379,7 @@ warning foo<br>
 </details>
 </details>
 `,
-		wantCondensedOut: `All passed.
+		wantCondensedOut: `All models passed.
 `,
 	}, {
 		name:                 "pyang with pass and fails",
@@ -512,8 +539,32 @@ Failed.
 		inValidatorResultDir: "testdata/oc-pyang-script-fail",
 		inValidatorId:        "oc-pyang",
 		wantPass:             false,
-		wantOut:              "Validator script failed -- infra bug?\n```\nI failed\n\n```",
-		wantCondensedOutSame: true,
+		wantOut: "Validator script failed -- infra bug?\n```\nI failed\n\n```" + `
+<details>
+  <summary>&#x2705;&nbsp; acl</summary>
+<details>
+  <summary>&#x2705;&nbsp; openconfig-acl</summary>
+Passed.
+</details>
+</details>
+<details>
+  <summary>&#x2705;&nbsp; optical-transport</summary>
+<details>
+  <summary>&#x2705;&nbsp; openconfig-optical-amplifier</summary>
+Passed.
+</details>
+<details>
+  <summary>&#x2705;&nbsp; openconfig-transport-line-protection</summary>
+Passed.
+<ul>
+  <pre>warning foo</pre>
+</ul>
+</details>
+</details>
+`,
+		wantCondensedOut: "Validator script failed -- infra bug?\n```\nI failed\n\n```" + `
+All models passed.
+`,
 	}, {
 		name:                 "openconfig-version, revision version, and .spec.yml checks all pass",
 		inValidatorResultDir: "testdata/misc-checks-pass",