Skip to content

Commit

Permalink
Merge pull request #45 from quarkus-qe/feature/improve-gh-pr-comment-…
Browse files Browse the repository at this point in the history 
…content

Improve GitHub PR comment content
  • Loading branch information
@michalvavrik
michalvavrik authored Sep 26, 2024
2 parents af15b85 + effdaa2 commit bd79c29
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 44 deletions.
172 changes: 141 additions & 31 deletions src/main/java/io/quarkus/qe/reporter/flakyrun/commentator/CreateGhPrComment.java
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
package io.quarkus.qe.reporter.flakyrun.commentator;

import io.quarkus.qe.reporter.flakyrun.reporter.FlakyRunReporter;
import io.quarkus.qe.reporter.flakyrun.FlakyReporterUtils;
import io.quarkus.qe.reporter.flakyrun.reporter.FlakyTest;
import io.quarkus.qe.reporter.flakyrun.summary.FlakyRunSummaryReporter;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import static io.quarkus.qe.reporter.flakyrun.FlakyReporterUtils.getRequiredArgument;
import static io.quarkus.qe.reporter.flakyrun.FlakyReporterUtils.readFile;
import static io.quarkus.qe.reporter.flakyrun.reporter.FlakyRunReporter.parseFlakyTestsReport;

/**
* This class is used by a Jbang script to simplify commenting in GitHub PRs when a flake were detected.
Expand All @@ -20,23 +25,68 @@ public final class CreateGhPrComment {
public static final String TEST_BASE_DIR = CreateGhPrComment.class.getSimpleName() + ".test-base-dir";
public static final String OVERVIEW_FILE_KEY = "overview-file";
public static final String FLAKY_REPORTS_FILE_PREFIX_KEY = "flaky-reports-file-prefix";
public static final String GH_REPO_ENV_VAR_NAME = "GH_REPO";
public static final String WORKFLOW_ID_ENV_VAR_NAME = "WORKFLOW_ID";
private static final Path CURRENT_DIR = Path.of(".");
private final String comment;
private final Path baseDir;

public CreateGhPrComment(String[] args) {
this(args, getRequiredEnv(GH_REPO_ENV_VAR_NAME), getRequiredEnv(WORKFLOW_ID_ENV_VAR_NAME));
}

public CreateGhPrComment(String[] args, String ghRepo, String workflowId) {
if (System.getProperty(TEST_BASE_DIR) != null) {
baseDir = Path.of(System.getProperty(TEST_BASE_DIR));
} else {
baseDir = CURRENT_DIR;
}
var failureOverview = getFailureOverview(args);
var flakyTestsReports = getFlakyTestReports(args);
var jobs = getJobs(args);
var failureOverview = getFailureOverview(jobs);
var flakyTestsReports = getFlakyTestReports(args, jobs);
var prNumber = getPrNumber();

this.comment = """
Following jobs contain at least one flaky test: %s
Following jobs contain at least one flaky test:
%s
Run summary: https://github.com/%s/actions/runs/%s?pr=%s
**Flaky tests:**
---
%s
""".formatted(failureOverview, flakyTestsReports);
""".formatted(failureOverview, ghRepo, workflowId, prNumber, flakyTestsReports);
}

private Set<String> getJobs(String[] args) {
// expected format:
// 'PR - Linux - JVM build - Latest Version', 'PR - Linux - Native build - Latest Version',
// 'PR - Windows - JVM build - Latest Version'
var overviewPath = baseDir.resolve(getRequiredArgument(OVERVIEW_FILE_KEY, args));
if (Files.exists(overviewPath)) {
var overview = readFile(overviewPath);
return Arrays.stream(overview.split(",")).map(String::trim).map(job -> {
if (job.startsWith("'")) {
return job.substring(1);
}
return job;
}).map(job -> {
if (job.endsWith("'")) {
return job.substring(0, job.length() - 1);
}
return job;
}).collect(Collectors.toSet());
}
throw new IllegalStateException("File '" + overviewPath + "' not found");
}

private String getPrNumber() {
var prNumber = FlakyReporterUtils.readFile(baseDir.resolve("pr-number"));
if (prNumber == null || prNumber.isBlank()) {
throw new IllegalStateException("File 'pr-number' not found, cannot proceed without the PR number");
}
return prNumber.trim();
}

public String getComment() {
Expand All @@ -47,43 +97,103 @@ public void printToStdOut() {
System.out.println(comment);
}

private String getFlakyTestReports(String[] args) {
private String getFlakyTestReports(String[] args, Set<String> jobs) {
var reportFilePrefix = getRequiredArgument(FLAKY_REPORTS_FILE_PREFIX_KEY, args);
var listOfDirFiles = baseDir.toFile().listFiles();
if (listOfDirFiles == null || listOfDirFiles.length == 0) {
return "No flaky test reports found";
}
Map<String, FlakyTestWithFiles> testNameToDetail = new HashMap<>();
var result = new StringBuilder();
for (File file : listOfDirFiles) {
if (file.getName().startsWith(reportFilePrefix)) {
var flakyTests = FlakyRunReporter.parseFlakyTestsReport(file.toPath());
if (!flakyTests.isEmpty()) {
result.append("**Artifact `%s` contains following failures:**".formatted(file.getName()));
for (FlakyTest flakyTest : flakyTests) {
result.append("""
- Test name: `%s`
Date and time: %s
Failure message: `%s`
Failure stacktrace:
```
%s
```
""".formatted(flakyTest.fullTestName(), flakyTest.dateTime(),
flakyTest.failureMessage(), flakyTest.failureStackTrace()));
}
result.append(System.lineSeparator());
}
// well, this is obviously imprecise in the sense that we expect stacktrace and failure message
// to be always same in all the runs, but here:
// https://github.com/quarkus-qe/quarkus-test-suite/pull/2050#issuecomment-2376769937
// it was requested that we list tests with list of jobs where they failed,
// and we cannot have both (stacktrace per each job and one stacktrace)
parseFlakyTestsReport(file.toPath()).forEach(flakyTest -> testNameToDetail
.computeIfAbsent(flakyTest.fullTestName(),
tn -> new FlakyTestWithFiles(new HashSet<>(), flakyTest))
.fileNames().add(file.getName()));
}
}

testNameToDetail.values().forEach(flakyTest -> result.append("""
**`%s`**
- Failure message: `%s`
- Failed in jobs:
%s
<details>
<summary>Failure stacktrace</summary>
```
%s
```
</details>
---
""".formatted(flakyTest.detail.fullTestName(), flakyTest.detail.failureMessage(),
toFailedInJobs(flakyTest.fileNames, jobs), flakyTest.detail.failureStackTrace())));

return result.toString();
}

private String getFailureOverview(String[] args) {
var overviewPath = baseDir.resolve(getRequiredArgument(OVERVIEW_FILE_KEY, args));
if (Files.exists(overviewPath)) {
return readFile(overviewPath);
private static String toFailedInJobs(Set<String> fileNames, Set<String> jobs) {
// produce:
// - ABC
// - EFG
return fileNames.stream().map(fileName -> toJobName(fileName, jobs)).map(jobName -> " - " + jobName)
.collect(Collectors.joining(System.lineSeparator()));
}

private static String toJobName(String fileName, Set<String> jobs) {
// this can be heavily simplified and made more precise
// by changing format we receive
// right now we receive something like:
// PR - Linux - JVM build - Latest Version
// flaky-run-report-linux-jvm-latest.json
// so the obvious intersection are the last 3 words
var words = fileName.transform(fn -> {
// drop .json
if (fn.endsWith(".json")) {
return fn.substring(0, fn.length() - 5);
}
return fn;
}).split("-");
if (words.length > 3) {
var last3Words = Arrays.stream(words).skip(words.length - 3).map(String::toLowerCase)
.collect(Collectors.toSet());
var jobName = jobs.stream().filter(job -> last3Words.stream().allMatch(w -> job.toLowerCase().contains(w)))
.findFirst().orElse(null);
if (jobName != null) {
return jobName.trim();
}
}
throw new IllegalStateException("File '" + overviewPath + "' not found");

// fallback to the filename
System.out.println("Unknown format for flaky report filename: " + fileName);
return fileName;
}

private static String getFailureOverview(Set<String> jobs) {
// produce:
// * PR - Linux - JVM build - Latest Version
// * PR - Linux - Native build - Latest Version
// * PR - Windows - JVM build - Latest Version
return jobs.stream().map(job -> " * " + job).collect(Collectors.joining(System.lineSeparator()));
}

private static String getRequiredEnv(String environmentVariableName) {
var envVar = System.getenv(environmentVariableName);
if (envVar == null) {
throw new IllegalArgumentException("Missing environment variable: " + environmentVariableName);
}
return envVar;
}

private record FlakyTestWithFiles(Set<String> fileNames, FlakyTest detail) {

}
}
38 changes: 25 additions & 13 deletions src/test/java/io/quarkus/qe/reporter/flakyrun/FlakyRunReportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import static io.quarkus.qe.reporter.flakyrun.summary.FlakyRunSummaryReporter.CI_BUILD_NUMBER;
import static io.quarkus.qe.reporter.flakyrun.summary.FlakyRunSummaryReporter.DAY_RETENTION;
import static io.quarkus.qe.reporter.flakyrun.summary.FlakyRunSummaryReporter.FLAKY_SUMMARY_REPORT;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand Down Expand Up @@ -65,6 +66,12 @@ private static void assertGitHubPrCommentator() throws IOException {
var testTarget = getFlakyRunReportFile().toPath().getParent();
System.setProperty(CreateGhPrComment.TEST_BASE_DIR, testTarget.toString());

// prepare PR Number
var prNumberFile = new File("src/test/resources/pr-number");
var prNumberFileInTestTarget = new File(TARGET_FLAKY_TEST_DIR, "target/" + prNumberFile.getName());
FileUtils.copyFile(prNumberFile, prNumberFileInTestTarget);
assertEquals("8888", readFile(prNumberFileInTestTarget.toPath()));

// prepare overview
var overview = new File("src/test/resources/overview_file.txt");
var overviewInTestTarget = new File(TARGET_FLAKY_TEST_DIR, "target/" + overview.getName());
Expand All @@ -73,27 +80,32 @@ private static void assertGitHubPrCommentator() throws IOException {

// prepare flaky run reports
var expectedReportPrefix = "flaky-run-report-";
var newFlakyRunReportFile1 = testTarget.resolve(expectedReportPrefix + "whatever-1").toFile();
var newFlakyRunReportFile2 = testTarget.resolve(expectedReportPrefix + "whatever-2").toFile();
var newFlakyRunReportFile1 = testTarget.resolve(expectedReportPrefix + "linux-jvm-latest.json").toFile();
var newFlakyRunReportFile2 = testTarget.resolve(expectedReportPrefix + "linux-native-latest.json").toFile();
var newFlakyRunReportFile3 = testTarget.resolve(expectedReportPrefix + "windows-jvm-latest.json").toFile();
FileUtils.copyFile(getFlakyRunReportFile(), newFlakyRunReportFile1);
FileUtils.copyFile(getFlakyRunReportFile(), newFlakyRunReportFile2);
FileUtils.copyFile(getFlakyRunReportFile(), newFlakyRunReportFile3);

// prepare comment
var commentator = new CreateGhPrComment(createCommandArgs(OVERVIEW_FILE_KEY, overview.getName(),
FLAKY_REPORTS_FILE_PREFIX_KEY, expectedReportPrefix));
var args = createCommandArgs(OVERVIEW_FILE_KEY, overview.getName(), FLAKY_REPORTS_FILE_PREFIX_KEY,
expectedReportPrefix);
var commentator = new CreateGhPrComment(args, "quarkus-qe/quarkus-test-suite", "1234567890");
var comment = commentator.getComment();

// assert comment
assertTrue(
comment.contains(
"Artifact `%s` contains following failures:".formatted(newFlakyRunReportFile1.getName())),
comment);
assertTrue(
comment.contains(
"Artifact `%s` contains following failures:".formatted(newFlakyRunReportFile2.getName())),
assertTrue(comment.contains("Following jobs contain at least one flaky test"), comment);
assertTrue(comment.contains(" * PR - Windows - JVM build - Latest Version"), comment);
assertTrue(comment.contains(" * PR - Linux - Native build - Latest Version"), comment);
assertTrue(comment.contains(" * PR - Linux - JVM build - Latest Version"), comment);
assertTrue(comment.contains(
"Run summary: https://github.com/quarkus-qe/quarkus-test-suite/actions/runs/1234567890?pr=8888"),
comment);
assertTrue(comment.contains("Failure message: `failing to test flakiness reporting`"), comment);
assertTrue(comment.contains("Failure stacktrace:"), comment);
assertTrue(comment.contains("**`io.quarkus.qe.reporter.flakyrun.FlakyTest.testFlaky`**"), comment);
assertTrue(comment.contains(" - Failure message: `failing to test flakiness reporting`"), comment);
assertTrue(comment.contains(" - PR - Windows - JVM build - Latest Version"), comment);
assertTrue(comment.contains(" - PR - Linux - JVM build - Latest Version"), comment);
assertTrue(comment.contains(" - PR - Linux - Native build - Latest Version"), comment);
assertTrue(comment.contains("org.opentest4j.AssertionFailedError: failing to test flakiness reporting"),
comment);
}
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/pr-number
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8888

0 comments on commit bd79c29

Please sign in to comment.