[K/N][test] Refactor test run checking

Make test filtering matching be a check instead of the verification code in ResultHandler. This allows turning it on/off for specific test cases and have different checking strategies.
2024-02-16 12:53:42 +01:00
parent 77f0cba23f
commit 8907885a75
10 changed files with 87 additions and 61 deletions
@@ -127,12 +127,8 @@ abstract class AbstractNativeCExportTest() : AbstractNativeSimpleTest() {
                "-opt-in", "kotlinx.cinterop.ExperimentalForeignApi",
            )),
            nominalPackageName = PackageName(moduleName),
-            checks = TestRunChecks(
-                executionTimeoutCheck = TestRunCheck.ExecutionTimeout.ShouldNotExceed(testRunSettings.get<Timeouts>().executionTimeout),
-                exitCodeCheck = TestRunCheck.ExitCode.Expected(0),
-                outputDataFile = goldenData?.let { TestRunCheck.OutputDataFile(file = it) },
-                outputMatcher = null,
-                fileCheckMatcher = null,
+            checks = TestRunChecks.Default(testRunSettings.get<Timeouts>().executionTimeout).copy(
+                outputDataFile = goldenData?.let { TestRunCheck.OutputDataFile(file = it) }
            ),
            extras = TestCase.NoTestRunnerExtras(entryPoint = "main")
        ).apply {
@@ -158,13 +158,7 @@ abstract class AbstractNativeSwiftExportTest() : AbstractNativeSimpleTest() {
                )
            ),
            nominalPackageName = PackageName(testName),
-            checks = TestRunChecks(
-                executionTimeoutCheck = TestRunCheck.ExecutionTimeout.ShouldNotExceed(testRunSettings.get<Timeouts>().executionTimeout),
-                exitCodeCheck = TestRunCheck.ExitCode.Expected(0),
-                outputDataFile = null,
-                outputMatcher = null,
-                fileCheckMatcher = null,
-            ),
+            checks = TestRunChecks.Default(testRunSettings.get<Timeouts>().executionTimeout),
            extras = TestCase.NoTestRunnerExtras(entryPoint = "main")
        ).apply {
            initialize(null, null)
@@ -246,12 +246,8 @@ class ObjCToKotlinSteppingInLLDBTest : AbstractNativeSimpleTest() {
            modules = setOf(moduleForTestCase),
            freeCompilerArgs = freeCompilerArgs,
            nominalPackageName = PackageName.EMPTY,
-            checks = TestRunChecks(
-                executionTimeoutCheck = ExecutionTimeout.ShouldNotExceed(testRunSettings.get<Timeouts>().executionTimeout),
-                exitCodeCheck = TestRunCheck.ExitCode.Expected(0),
-                outputDataFile = null,
-                outputMatcher = spec.let { TestRunCheck.OutputMatcher { output -> spec.checkLLDBOutput(output, testRunSettings.get()) } },
-                fileCheckMatcher = null,
+            checks = TestRunChecks.Default(testRunSettings.get<Timeouts>().executionTimeout).copy(
+                outputMatcher = spec.let { TestRunCheck.OutputMatcher { output -> spec.checkLLDBOutput(output, testRunSettings.get()) } }
            ),
            extras = TestCase.NoTestRunnerExtras(
                "main",
@@ -539,9 +539,10 @@ private class ExtTestDataFile(
            nominalPackageName = testDataFileSettings.nominalPackageName,
            expectedFailure = isExpectedFailure,
            checks = TestRunChecks.Default(timeouts.executionTimeout).copy(
-                fileCheckMatcher = fileCheckStage?.let { TestRunCheck.FileCheckMatcher(settings, testDataFile) },
+                testFiltering = TestRunCheck.TestFiltering(TCTestOutputFilter),
                // for expected failures, it does not matter, which exit code would the process have, since test might fail with other reasons
                exitCodeCheck = TestRunCheck.ExitCode.Expected(0).takeUnless { isExpectedFailure },
+                fileCheckMatcher = fileCheckStage?.let { TestRunCheck.FileCheckMatcher(settings, testDataFile) }
            ),
            fileCheckStage = fileCheckStage,
            extras = WithTestRunnerExtras(runnerType = TestRunnerType.DEFAULT)
@@ -7,10 +7,12 @@ package org.jetbrains.kotlin.konan.test.blackbox.support.group

 import org.jetbrains.kotlin.konan.test.blackbox.support.*
 import org.jetbrains.kotlin.konan.test.blackbox.support.TestCase.WithTestRunnerExtras
+import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunCheck
 import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunChecks
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.KotlinNativeHome
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.Settings
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.Timeouts
+import org.jetbrains.kotlin.konan.test.blackbox.support.util.TCTestOutputFilter
 import org.jetbrains.kotlin.konan.test.blackbox.support.util.ThreadSafeCache
 import org.jetbrains.kotlin.konan.test.blackbox.support.util.expandGlobTo
 import org.jetbrains.kotlin.konan.test.blackbox.support.util.getAbsoluteFile
@@ -66,7 +68,8 @@ internal class PredefinedTestCaseGroupProvider(annotation: PredefinedTestCases)
                freeCompilerArgs = predefinedTestCase.freeCompilerArgs
                    .parseCompilerArgs(settings) { "Failed to parse free compiler arguments for test case $testCaseId" },
                nominalPackageName = PackageName(testCaseId.uniqueName),
-                checks = TestRunChecks.Default(settings.get<Timeouts>().executionTimeout),
+                checks = TestRunChecks.Default(settings.get<Timeouts>().executionTimeout)
+                    .copy(testFiltering = TestRunCheck.TestFiltering(TCTestOutputFilter)),
                extras = WithTestRunnerExtras(
                    runnerType = predefinedTestCase.runnerType,
                    ignoredTests = predefinedTestCase.ignoredTests.toSet()
@@ -10,6 +10,8 @@ import org.jetbrains.kotlin.konan.test.blackbox.support.TestCase.NoTestRunnerExt
 import org.jetbrains.kotlin.konan.test.blackbox.support.TestCase.WithTestRunnerExtras
 import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunCheck.*
 import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunChecks
+import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunParameter
+import org.jetbrains.kotlin.konan.test.blackbox.support.runner.has
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.*
 import org.jetbrains.kotlin.konan.test.blackbox.support.util.*
 import org.jetbrains.kotlin.test.directives.model.Directive
@@ -225,6 +227,7 @@ internal class StandardTestCaseGroupProvider : TestCaseGroupProvider {
            expectedFailure = settings.isIgnoredTarget(registeredDirectives),
            checks = TestRunChecks(
                computeExecutionTimeoutCheck(settings, expectedTimeoutFailure),
+                computeTestOutputFiltering(testKind),
                computeExitCodeCheck(testKind, registeredDirectives, location),
                computeOutputDataFileCheck(testDataFile, registeredDirectives, location),
                outputMatcher,
@@ -301,5 +304,9 @@ internal class StandardTestCaseGroupProvider : TestCaseGroupProvider {
            registeredDirectives: RegisteredDirectives,
            location: Location
        ): OutputDataFile? = parseOutputDataFile(baseDir = testDataFile.parentFile, registeredDirectives, location)
+
+        private fun computeTestOutputFiltering(testKind: TestKind): TestFiltering = TestFiltering(
+            if (testKind in listOf(TestKind.REGULAR, TestKind.STANDALONE)) TCTestOutputFilter else TestOutputFilter.NO_FILTERING
+        )
    }
 }
@@ -12,7 +12,7 @@ import org.jetbrains.kotlin.konan.test.blackbox.support.TestName
 import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunCheck.ExecutionTimeout
 import org.jetbrains.kotlin.konan.test.blackbox.support.runner.TestRunCheck.ExitCode
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.configurables
-import org.jetbrains.kotlin.konan.test.blackbox.support.util.TestReport
+import org.jetbrains.kotlin.konan.test.blackbox.support.util.TestOutputFilter
 import org.jetbrains.kotlin.native.executors.RunProcessResult
 import org.jetbrains.kotlin.native.executors.runProcess
 import org.junit.jupiter.api.Assumptions
@@ -39,10 +39,10 @@ internal class ResultHandler(
        val diagnostics = buildList<String> {
            checks.forEach { check ->
                when (check) {
-                    is ExecutionTimeout.ShouldNotExceed -> if(!runResult.hasFinishedOnTime) add(
+                    is ExecutionTimeout.ShouldNotExceed -> if (!runResult.hasFinishedOnTime) add(
                        "Timeout exceeded during test execution."
                    )
-                    is ExecutionTimeout.ShouldExceed -> if(runResult.hasFinishedOnTime) add(
+                    is ExecutionTimeout.ShouldExceed -> if (runResult.hasFinishedOnTime) add(
                        "Test is expected to fail with exceeded timeout, which hasn't happened."
                    )
                    is ExitCode -> {
@@ -63,13 +63,13 @@ internal class ResultHandler(

                        // Don't use verifyExpectation(expected, actual) to avoid exposing potentially large test output in exception message
                        // and blowing up test logs.
-                        if(convertLineSeparators(expectedOutput) != convertLineSeparators(actualFilteredOutput)) add(
+                        if (convertLineSeparators(expectedOutput) != convertLineSeparators(actualFilteredOutput)) add(
                            "Tested process output mismatch. See \"TEST STDOUT\" and \"EXPECTED OUTPUT DATA FILE\" below."
                        )
                    }
                    is TestRunCheck.OutputMatcher -> {
                        try {
-                            if(!check.match(runResult.processOutputAsString(check.output))) add(
+                            if (!check.match(runResult.processOutputAsString(check.output))) add(
                                "Tested process output has not passed validation."
                            )
                        } catch (t: Throwable) {
@@ -87,7 +87,8 @@ internal class ResultHandler(
                        if (!(result.stdout.isEmpty() && result.stderr.isEmpty())) {
                            val shortOutText = result.stdout.lines().take(100)
                            val shortErrText = result.stderr.lines().take(100)
-                            add("FileCheck matching of ${fileCheckDump.absolutePath}\n" +
+                            add(
+                                "FileCheck matching of ${fileCheckDump.absolutePath}\n" +
                                        "with '--check-prefixes ${check.prefixes}'\n" +
                                        "failed with result=$result:\n" +
                                        shortOutText.joinToString("\n") + "\n" +
@@ -95,6 +96,37 @@ internal class ResultHandler(
                            )
                        }
                    }
+                    is TestRunCheck.TestFiltering -> {
+                        if (check.testOutputFilter != TestOutputFilter.NO_FILTERING) {
+                            val testReport = runResult.processOutput.stdOut.testReport
+
+                            if (testReport == null) {
+                                add("TestRun has TestFiltering enabled, but test report is null")
+                            }
+                            checkNotNull(testReport)
+
+                            if (testReport.isEmpty()) add("No tests have been found. Test report is empty")
+
+                            testRun.runParameters.get<TestRunParameter.WithFilter> {
+                                verifyNoSuchTests(
+                                    testReport.passedTests.filter { testName -> !testMatches(testName) },
+                                    "Excessive tests have been executed"
+                                )
+
+                                verifyNoSuchTests(
+                                    testReport.ignoredTests.filter { testName -> !testMatches(testName) },
+                                    "Excessive tests have been ignored"
+                                )
+                            }
+
+                            verifyNoSuchTests(testReport.failedTests, "Failed tests found in the test report")
+
+                            Assumptions.assumeFalse(
+                                testReport.ignoredTests.isNotEmpty() && testReport.passedTests.isEmpty(),
+                                "Test case is disabled"
+                            )
+                        }
+                    }
                }
            }
        }
@@ -103,12 +135,16 @@ internal class ResultHandler(
                diagnostics.joinToString("\n")
            }
        } else {
-            val runResultInfo = "TestCaseId: ${testRun.testCase.id}\nExit code: ${runResult.exitCode}\nFiltered test output is${
-                runResult.processOutput.stdOut.filteredOutput.let {
+            val runResultInfo = buildString {
+                appendLine("TestCaseId: ${testRun.testCase.id}")
+                appendLine("Exit code: ${runResult.exitCode}")
+                appendLine("Filtered test output is")
+                appendLine(runResult.processOutput.stdOut.filteredOutput.let {
                    if (it.isNotEmpty()) ":\n$it" else " empty."
-                }
-            }"
-            verifyExpectation(diagnostics.isNotEmpty() || runResult.processOutput.stdOut.testReport?.failedTests?.isNotEmpty() == true) {
+                })
+                appendLine(runResult.processOutput.stdOut.testReport)
+            }
+            verifyExpectation(diagnostics.isNotEmpty()) {
                "Test did not fail as expected: $runResultInfo"
            }
            println("Test failed as expected.\n$runResultInfo")
@@ -117,36 +153,16 @@ internal class ResultHandler(
                diagnostics.forEach(::println)
            }
        }
-
-        verifyTestReport(runResult.processOutput.stdOut.testReport)
    }

-    private fun verifyTestReport(testReport: TestReport?) {
-        if (testReport == null) return
-
-        verifyExpectation(!testReport.isEmpty()) { "No tests have been found." }
-
-        testRun.runParameters.get<TestRunParameter.WithFilter> {
-            verifyNoSuchTests(
-                testReport.passedTests.filter { testName -> !testMatches(testName) },
-                "Excessive tests have been executed"
+    private fun MutableList<String>.verifyNoSuchTests(tests: Collection<TestName>, subject: String) {
+        if (tests.isNotEmpty()) {
+            add(
+                buildString {
+                    append(subject).append(':')
+                    tests.forEach { appendLine().append(" - ").append(it) }
+                }
            )
-
-            verifyNoSuchTests(
-                testReport.ignoredTests.filter { testName -> !testMatches(testName) },
-                "Excessive tests have been ignored"
-            )
-        }
-
-        if (!testRun.expectedFailure)
-            verifyNoSuchTests(testReport.failedTests, "There are failed tests")
-        Assumptions.assumeFalse(testReport.ignoredTests.isNotEmpty() && testReport.passedTests.isEmpty(), "Test case is disabled")
-    }
-
-    private fun verifyNoSuchTests(tests: Collection<TestName>, subject: String) = verifyExpectation(tests.isEmpty()) {
-        buildString {
-            append(subject).append(':')
-            tests.forEach { appendLine().append(" - ").append(it) }
        }
    }
 }
@@ -24,7 +24,7 @@ internal open class RunnerWithExecutor(
    private val executable get() = testRun.executable

    private val outputFilter: TestOutputFilter
-        get() = if (testRun.runParameters.has<TestRunParameter.WithTCTestLogger>()) TCTestOutputFilter else TestOutputFilter.NO_FILTERING
+        get() = testRun.checks.testFiltering.testOutputFilter

    private val programArgs = mutableListOf<String>().apply {
        add(executable.executable.executableFile.absolutePath)
@@ -13,6 +13,7 @@ import org.jetbrains.kotlin.konan.test.blackbox.support.settings.CacheMode
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.KotlinNativeTargets
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.OptimizationMode
 import org.jetbrains.kotlin.konan.test.blackbox.support.settings.Settings
+import org.jetbrains.kotlin.konan.test.blackbox.support.util.TestOutputFilter
 import org.jetbrains.kotlin.util.capitalizeDecapitalize.toUpperCaseAsciiOnly
 import org.jetbrains.kotlin.utils.yieldIfNotNull
 import java.io.File
@@ -41,7 +42,9 @@ internal sealed interface TestRunCheck {

    class OutputDataFile(val output: Output = Output.ALL, val file: File) : TestRunCheck

-    class OutputMatcher(val output: Output = Output.ALL, val match: (String) -> Boolean): TestRunCheck
+    class OutputMatcher(val output: Output = Output.ALL, val match: (String) -> Boolean) : TestRunCheck
+
+    class TestFiltering(val testOutputFilter: TestOutputFilter) : TestRunCheck

    class FileCheckMatcher(val settings: Settings, val testDataFile: File) : TestRunCheck {
        val prefixes: String
@@ -71,6 +74,7 @@ internal sealed interface TestRunCheck {

 internal data class TestRunChecks(
    val executionTimeoutCheck: ExecutionTimeout,
+    val testFiltering: TestFiltering,
    private val exitCodeCheck: ExitCode?,
    val outputDataFile: OutputDataFile?,
    val outputMatcher: OutputMatcher?,
@@ -79,6 +83,7 @@ internal data class TestRunChecks(

    override fun iterator() = iterator {
        yield(executionTimeoutCheck)
+        yield(testFiltering)
        yieldIfNotNull(exitCodeCheck)
        yieldIfNotNull(outputDataFile)
        yieldIfNotNull(outputMatcher)
@@ -90,6 +95,7 @@ internal data class TestRunChecks(
        @Suppress("TestFunctionName")
        fun Default(timeout: Duration) = TestRunChecks(
            executionTimeoutCheck = ExecutionTimeout.ShouldNotExceed(timeout),
+            testFiltering = TestFiltering(TestOutputFilter.NO_FILTERING),
            exitCodeCheck = ExitCode.Expected(0),
            outputDataFile = null,
            outputMatcher = null,
@@ -17,6 +17,13 @@ internal class TestReport(
    val ignoredTests: Collection<TestName>
 ) {
    fun isEmpty(): Boolean = passedTests.isEmpty() && failedTests.isEmpty() && ignoredTests.isEmpty()
+
+    override fun toString(): String = """
+        TestReport:
+         * Passed:  $passedTests
+         * Failed:  $failedTests
+         * Ignored: $ignoredTests
+    """.trimIndent()
 }

 internal interface TestOutputFilter {