Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve GitHub PR comment content #45

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {

}
}
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8888
Loading