add full report to artifact

.github/workflows/conformance-report.yml

@@ -5,6 +5,8 @@ env:
   PATH_TO_TEST_RUNNER: test/partiql-tests-runner
   CONFORMANCE_REPORT_RELATIVE_PATH: build/conformance-test-report
   COMPARISON_REPORT_NAME: comparison_report.md
+  COMPARISON_REPORT_NAME_WITH_LIMIT: comparison_report_limited.md
+  COMMENT_SIZE_LIMIT: 10
 
 jobs:
   conformance-report:
@@ -28,7 +30,7 @@ jobs:
       # Run the conformance tests and save to an Ion file.
       - name: gradle test of the conformance tests (can fail) and save to Ion file
         continue-on-error: true
-        run: gradle :test:partiql-tests-runner:ConformanceTestReport
+        run: gradle :test:partiql-tests-runner:generateTestReport
       # Upload conformance report for future viewing and comparison with future runs.
       - name: Upload `conformance-test-report` folder
         uses: actions/upload-artifact@v3
@@ -87,23 +89,34 @@ jobs:
         continue-on-error: true
         run: |
           cd ${{ github.event.pull_request.base.sha }}
-          gradle :test:partiql-tests-runner:ConformanceTestReport
+          gradle :test:partiql-tests-runner:generateTestReport
       - name: (If download of target branch conformance report fails) Move conformance test report of target branch to ./artifact directory
         if: ${{ steps.download-report.outcome == 'failure' }}
         continue-on-error: true
         run: |
           mkdir -p $GITHUB_WORKSPACE/artifact
           cp -r $GITHUB_WORKSPACE/${{ github.event.pull_request.base.sha }}/$PATH_TO_TEST_RUNNER/$CONFORMANCE_REPORT_RELATIVE_PATH $GITHUB_WORKSPACE/artifact/$CONFORMANCE_REPORT_RELATIVE_PATH
       # Run conformance report comparison. Generates `comparison_report.md`
-      - name: Run conformance report comparison. Generates `comparison_report.md`
+      - name: Run conformance report comparison for artifact. Generates `comparison_report.md`
         continue-on-error: true
         run: |
           ARGS="$GITHUB_WORKSPACE/artifact $CONFORMANCE_REPORT_RELATIVE_PATH ${{ github.event.pull_request.base.sha }} $GITHUB_SHA $COMPARISON_REPORT_NAME"
           gradle :test:partiql-tests-runner:run --args="$ARGS"
       # Print conformance report to GitHub actions workflow summary page
       - name: Print markdown in run
         continue-on-error: true
-        run: echo $PATH_TO_TEST_RUNNER/$COMPARISON_REPORT_NAME >> $GITHUB_STEP_SUMMARY
+        run: cat $PATH_TO_TEST_RUNNER/$COMPARISON_REPORT_NAME >> $GITHUB_STEP_SUMMARY
+      # Upload the full comparison report to CI artifact
+      - name: Upload `comparison_report.md`
+        uses: actions/upload-artifact@v3
+        with:
+          path: ${{ env.PATH_TO_TEST_RUNNER }}/comparison_report.md
+      # Rebuild the test report with a size limit for comment
+      - name: Run conformance report comparison for comment. Generates `comparison_report_limited.md`
+        continue-on-error: true
+        run: |
+          ARGS="$GITHUB_WORKSPACE/artifact $CONFORMANCE_REPORT_RELATIVE_PATH ${{ github.event.pull_request.base.sha }} $GITHUB_SHA $COMPARISON_REPORT_NAME_WITH_LIMIT $COMMENT_SIZE_LIMIT"
+          gradle :test:partiql-tests-runner:run --args="$ARGS"
       # Find comment w/ conformance comparison if previous comment published
       - name: Find Comment
         uses: peter-evans/find-comment@v2
@@ -120,5 +133,5 @@ jobs:
         with:
           comment-id: ${{ steps.fc.outputs.comment-id }}
           issue-number: ${{ github.event.pull_request.number }}
-          body-file: ${{ env.PATH_TO_TEST_RUNNER }}/${{ env.COMPARISON_REPORT_NAME }}
+          body-file: ${{ env.PATH_TO_TEST_RUNNER }}/${{ env.COMPARISON_REPORT_NAME_WITH_LIMIT }}
           edit-mode: replace

test/partiql-tests-runner/build.gradle.kts

@@ -31,6 +31,7 @@ dependencies {
 }
 
 val tests = System.getenv()["PARTIQL_TESTS_DATA"] ?: "../partiql-tests/partiql-tests-data"
+val reportDir = file("$buildDir/conformance-test-report").absolutePath
 
 object Env {
     const val PARTIQL_EVAL = "PARTIQL_EVAL_TESTS_DATA"
@@ -49,16 +50,19 @@ tasks.test {
     exclude("org/partiql/runner/ConformanceTest.class")
 }
 
-tasks.register<Test>("ConformanceTestReport") {
-    val reportDir = file("$buildDir/conformance-test-report").absolutePath
+val createReportDir by tasks.registering {
     if (File(reportDir).exists()) {
         delete(File(reportDir))
     }
+    mkdir(reportDir)
+}
+
+val generateTestReport by tasks.registering(Test::class) {
+    dependsOn(createReportDir)
     useJUnitPlatform()
     environment(Env.PARTIQL_EVAL, file("$tests/eval/").absolutePath)
     environment(Env.PARTIQL_EQUIV, file("$tests/eval-equiv/").absolutePath)
     environment("conformanceReportDir", reportDir)
-    mkdir(reportDir)
     include("org/partiql/runner/ConformanceTestEval.class", "org/partiql/runner/ConformanceTestLegacy.class")
     if (project.hasProperty("Engine")) {
         val engine = property("Engine")!! as String

test/partiql-tests-runner/src/main/kotlin/org/partiql/runner/ConformanceComparison.kt

@@ -4,9 +4,9 @@ import com.amazon.ionelement.api.loadSingleElement
 import java.io.File
 
 fun main(args: Array<String>) {
-    if (args.size != 5) {
+    if (args.size < 5) {
         error(
-            "Expected 5 args: pathToFirstConformanceTestResults, pathToSecondConformanceTestResults" +
+            "Expected at least 5 args: pathToFirstConformanceTestResults, pathToSecondConformanceTestResults" +
                 "firstCommitId, secondCommitId, pathToComparisonReport"
         )
     }
@@ -20,18 +20,20 @@ fun main(args: Array<String>) {
     val oldReports = loadReport(old, oldCommitId)
     val newReports = loadReport(new, newCommitId)
     val comparisonReportFile = File(args[4])
+    val limit = if (args.size == 6) args[5].toInt() else Int.MAX_VALUE
 
     comparisonReportFile.createNewFile()
 
-    analyze(comparisonReportFile, newReports)
+    // cross engine
+    analyze(comparisonReportFile, newReports, limit)
 
     val all = oldReports + newReports
 
-    // cross commit comparsion
+    // cross commit comparison
     all
         .groupBy { it.engine }
         .forEach { (_, reports) ->
-            analyze(comparisonReportFile, reports)
+            analyze(comparisonReportFile, reports, limit)
         }
 }
 data class Report(
@@ -42,11 +44,11 @@ data class Report(
     val ignoredSet: Set<String>
 )
 
-fun analyze(file: File, reports: List<Report>) {
+fun analyze(file: File, reports: List<Report>, limit: Int) {
     var first = 0
     var second = first + 1
     while (first < second && second < reports.size) {
-        val report = ReportAnalyzer.build(reports[first], reports[second]).generateComparisonReport()
+        val report = ReportAnalyzer.build(reports[first], reports[second]).generateComparisonReport(limit)
         file.appendText(report)
         file.appendText("\n")
         if (second < reports.size - 1) {
@@ -59,12 +61,14 @@ fun analyze(file: File, reports: List<Report>) {
 }
 
 fun loadReport(dir: File, commitId: String) =
-    dir.listFiles()?.map { sub ->
-        val engine = sub.name
-        val report =
-            (sub.listFiles() ?: throw IllegalArgumentException("sub-dir ${sub.absolutePath} not exist")).first().readText()
-        loadReport(report, engine, commitId)
-    } ?: throw IllegalArgumentException("dir ${dir.absolutePath} not exist")
+    dir.listFiles()
+        ?.filter { it.isDirectory }
+        ?.map { sub ->
+            val engine = sub.name
+            val report =
+                (sub.listFiles() ?: throw IllegalArgumentException("sub-dir ${sub.absolutePath} not exist")).first().readText()
+            loadReport(report, engine, commitId)
+        } ?: throw IllegalArgumentException("dir ${dir.absolutePath} not exist")
 
 fun loadReport(report: String, engine: String, commitId: String): Report {
     val inputStruct = loadSingleElement(report).asStruct()
@@ -101,13 +105,13 @@ abstract class ReportAnalyzer(first: Report, second: Report) {
     val secondPassingPercent = secondPassingSize.toDouble() / secondTotalSize * 100
 
     abstract val reportTitle: String
-    abstract fun generateComparisonReport(): String
+    abstract fun generateComparisonReport(limit: Int): String
 }
 
 class CrossCommitReportAnalyzer(private val first: Report, private val second: Report) :
     ReportAnalyzer(first, second) {
     override val reportTitle: String = "Conformance comparison report-Cross Commit-${first.engine.uppercase()}"
-    override fun generateComparisonReport() =
+    override fun generateComparisonReport(limit: Int) =
         buildString {
             this.appendLine(
                 """### $reportTitle
@@ -134,7 +138,7 @@ Number failing in Base (${first.commitId}) but now pass: ${failureFirstPassingSe
             if (passingFirstFailingSecond.isNotEmpty()) {
                 // character count limitation with comments in GitHub
                 // also, not ideal to list out hundreds of test names
-                if (passingFirstFailingSecond.size < 10) {
+                if (passingFirstFailingSecond.size < limit) {
                     this.appendLine(":interrobang: CONFORMANCE REPORT REGRESSION DETECTED :interrobang:. The following test(s) were previously passing but now fail:\n<details><summary>Click here to see</summary>\n\n")
 
                     passingFirstFailingSecond.forEach { testName ->
@@ -143,11 +147,12 @@ Number failing in Base (${first.commitId}) but now pass: ${failureFirstPassingSe
                     this.appendLine("</details>")
                 } else {
                     this.appendLine(":interrobang: CONFORMANCE REPORT REGRESSION DETECTED :interrobang:")
+                    this.appendLine("The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.")
                 }
             }
 
             if (failureFirstPassingSecond.isNotEmpty()) {
-                if (failureFirstPassingSecond.size < 10) {
+                if (failureFirstPassingSecond.size < limit) {
                     this.appendLine(
                         "The following test(s) were previously failing but now pass. Before merging, confirm they are intended to pass: \n<details><summary>Click here to see</summary>\n\n"
                     )
@@ -157,14 +162,15 @@ Number failing in Base (${first.commitId}) but now pass: ${failureFirstPassingSe
                     this.appendLine("</details>")
                 } else {
                     this.appendLine("${failureFirstPassingSecond.size} test(s) were previously failing but now pass. Before merging, confirm they are intended to pass")
+                    this.appendLine("The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.")
                 }
             }
         }
 }
 
 class CrossEngineReportAnalyzer(private val first: Report, private val second: Report) : ReportAnalyzer(first, second) {
     override val reportTitle: String = "Conformance comparison report-Cross Engine"
-    override fun generateComparisonReport() =
+    override fun generateComparisonReport(limit: Int) =
         buildString {
             this.appendLine(
                 """### $reportTitle
@@ -189,7 +195,7 @@ Number failing in ${first.engine} engine but pass in ${second.engine} engine: ${
                 """.trimIndent()
             )
             if (passingFirstFailingSecond.isNotEmpty()) {
-                if (passingFirstFailingSecond.size < 10) {
+                if (passingFirstFailingSecond.size < limit) {
                     this.appendLine(":interrobang: CONFORMANCE REPORT REGRESSION DETECTED :interrobang:. The following test(s) are passing in ${first.engine} but fail in ${second.engine}:\n<details><summary>Click here to see</summary>\n\n")
 
                     passingFirstFailingSecond.forEach { testName ->
@@ -198,11 +204,12 @@ Number failing in ${first.engine} engine but pass in ${second.engine} engine: ${
                     this.appendLine("</details>")
                 } else {
                     this.appendLine(":interrobang: CONFORMANCE REPORT REGRESSION DETECTED :interrobang:")
+                    this.appendLine("The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.")
                 }
             }
 
             if (failureFirstPassingSecond.isNotEmpty()) {
-                if (failureFirstPassingSecond.size < 10) {
+                if (failureFirstPassingSecond.size < limit) {
                     this.appendLine(
                         "The following test(s) are failing in ${first.engine} but pass in ${second.engine}. Before merging, confirm they are intended to pass: \n<details><summary>Click here to see</summary>\n\n"
                     )
@@ -212,6 +219,7 @@ Number failing in ${first.engine} engine but pass in ${second.engine} engine: ${
                     this.appendLine("</details>")
                 } else {
                     this.appendLine("${failureFirstPassingSecond.size} test(s) were failing in ${first.engine} but now pass in ${second.engine}. Before merging, confirm they are intended to pass.")
+                    this.appendLine("The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.")
                 }
             }
         }