From effdaa299d40741ee6b5dfb079183b1422c933f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Vav=C5=99=C3=ADk?= Date: Thu, 26 Sep 2024 22:15:39 +0200 Subject: [PATCH] Improve GitHub PR comment content --- .../commentator/CreateGhPrComment.java | 172 ++++++++++++++---- .../reporter/flakyrun/FlakyRunReportTest.java | 38 ++-- src/test/resources/pr-number | 1 + 3 files changed, 167 insertions(+), 44 deletions(-) create mode 100644 src/test/resources/pr-number diff --git a/src/main/java/io/quarkus/qe/reporter/flakyrun/commentator/CreateGhPrComment.java b/src/main/java/io/quarkus/qe/reporter/flakyrun/commentator/CreateGhPrComment.java index 743ddea..7da1c7f 100644 --- a/src/main/java/io/quarkus/qe/reporter/flakyrun/commentator/CreateGhPrComment.java +++ b/src/main/java/io/quarkus/qe/reporter/flakyrun/commentator/CreateGhPrComment.java @@ -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. @@ -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 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() { @@ -47,43 +97,103 @@ public void printToStdOut() { System.out.println(comment); } - private String getFlakyTestReports(String[] args) { + private String getFlakyTestReports(String[] args, Set 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 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 +
+ Failure stacktrace + + ``` + %s + ``` + +
+ + --- + """.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 fileNames, Set 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 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 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 fileNames, FlakyTest detail) { + } } diff --git a/src/test/java/io/quarkus/qe/reporter/flakyrun/FlakyRunReportTest.java b/src/test/java/io/quarkus/qe/reporter/flakyrun/FlakyRunReportTest.java index 4c90f59..df5766f 100644 --- a/src/test/java/io/quarkus/qe/reporter/flakyrun/FlakyRunReportTest.java +++ b/src/test/java/io/quarkus/qe/reporter/flakyrun/FlakyRunReportTest.java @@ -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; @@ -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()); @@ -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); } diff --git a/src/test/resources/pr-number b/src/test/resources/pr-number new file mode 100644 index 0000000..ecbc017 --- /dev/null +++ b/src/test/resources/pr-number @@ -0,0 +1 @@ +8888 \ No newline at end of file