From 151244468a1f1154a279193b43529e2fbc0fce01 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 25 Apr 2018 18:17:44 +0200 Subject: [PATCH 1/8] When a test fails report number of _failures_ not _errors_ (V2) Right now we report the number of "errors", that is compiler errors including the expected ones, but we should instead use the test "failures", that is the unexpected behaviors. Also make errorCount private. --- .../test/dotty/tools/vulpix/ParallelTesting.scala | 11 ++++++----- .../test/dotty/tools/vulpix/VulpixUnitTests.scala | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index e84724f83c39..2ec64470132a 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -227,7 +227,8 @@ trait ParallelTesting extends RunnerOrchestration { self => val sourceCount = filteredSources.length private[this] var _errorCount = 0 - def errorCount: Int = _errorCount + /** Number of (expected or unexpected) compiler errors. Unlikely to be what you want, use `failureCount` instead. */ + private[this] def errorCount: Int = _errorCount private[this] var _testSourcesCompleted = 0 private def testSourcesCompleted: Int = _testSourcesCompleted @@ -1012,11 +1013,11 @@ trait ParallelTesting extends RunnerOrchestration { self => /** Extract `Failure` set and render from `Test` */ private[this] def reasonsForFailure(test: Test): String = { - val errors = - if (test.errorCount == 0) "" - else s"\n - encountered ${test.errorCount} error(s)" + val failureReport = + if (test.failureCount == 0) "" + else s"\n - encountered ${test.failureCount} test failures(s)" - errors + test.failureReasons.collect { + failureReport + test.failureReasons.collect { case test.TimeoutFailure(title) => s" - test '$title' timed out" case test.JavaCompilationFailure(msg) => diff --git a/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala b/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala index 50cc51b4fccb..1152a15aa594 100644 --- a/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala +++ b/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala @@ -91,7 +91,7 @@ class VulpixUnitTests extends ParallelTesting { fail() } catch { case ae: AssertionError => - assertEquals(ae.getMessage, "Run test failed, but should not, reasons:\n - test 'tests/vulpix-tests/unit/timeout.scala' timed out") + assertEquals(ae.getMessage, "Run test failed, but should not, reasons:\n\n - encountered 1 test failures(s) - test 'tests/vulpix-tests/unit/timeout.scala' timed out") } } } From a8c2e51c4c3cf8ad10b567a02fe5179ad588be2d Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 25 Apr 2018 18:22:52 +0200 Subject: [PATCH 2/8] Remove unused _errorCount field --- compiler/test/dotty/tools/vulpix/ParallelTesting.scala | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 2ec64470132a..a23ed02b2f92 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -226,17 +226,12 @@ trait ParallelTesting extends RunnerOrchestration { self => /** Total amount of test sources being compiled by this test */ val sourceCount = filteredSources.length - private[this] var _errorCount = 0 - /** Number of (expected or unexpected) compiler errors. Unlikely to be what you want, use `failureCount` instead. */ - private[this] def errorCount: Int = _errorCount - private[this] var _testSourcesCompleted = 0 private def testSourcesCompleted: Int = _testSourcesCompleted /** Complete the current compilation with the amount of errors encountered */ protected final def registerCompletion(errors: Int) = synchronized { _testSourcesCompleted += 1 - _errorCount += errors } sealed trait Failure From b3e4c937f591f3ee2ebb1e7c9285f3ada22def47 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 25 Apr 2018 18:24:05 +0200 Subject: [PATCH 3/8] Stop counting compiler errors since we don't need this number --- .../test/dotty/tools/vulpix/ParallelTesting.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index a23ed02b2f92..b744f8227303 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -230,7 +230,7 @@ trait ParallelTesting extends RunnerOrchestration { self => private def testSourcesCompleted: Int = _testSourcesCompleted /** Complete the current compilation with the amount of errors encountered */ - protected final def registerCompletion(errors: Int) = synchronized { + protected final def registerCompletion() = synchronized { _testSourcesCompleted += 1 } @@ -323,7 +323,7 @@ trait ParallelTesting extends RunnerOrchestration { self => // run should fail failTestSource(testSource) e.printStackTrace() - registerCompletion(1) + registerCompletion() throw e } } @@ -551,7 +551,7 @@ trait ParallelTesting extends RunnerOrchestration { self => } else if (fromTasty) compileFromTasty(flags, false, outDir) else compile(testSource.sourceFiles, flags, false, outDir) - registerCompletion(reporter.errorCount) + registerCompletion() if (reporter.compilerCrashed || reporter.errorCount > 0) { logReporterContents(reporter) @@ -570,7 +570,7 @@ trait ParallelTesting extends RunnerOrchestration { self => def warningCount = reporters.foldLeft(0)(_ + _.warningCount) - registerCompletion(errorCount) + registerCompletion() if (compilerCrashed || errorCount > 0) { reporters.foreach(logReporterContents) @@ -691,7 +691,7 @@ trait ParallelTesting extends RunnerOrchestration { self => addFailureInstruction(buildInstr) failTestSource(testSource) } - registerCompletion(errorCount) + registerCompletion() } } } @@ -794,7 +794,7 @@ trait ParallelTesting extends RunnerOrchestration { self => else if (!errorMap.isEmpty) fail(s"\nExpected error(s) have {=}: $errorMap") - registerCompletion(actualErrors) + registerCompletion() } } } From aa6652e7c63aca4b3adc9bfcf984dfb02cc93f50 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 25 Apr 2018 21:54:29 +0200 Subject: [PATCH 4/8] Update sbt-output.check for (disabled by default) vulpix meta tests --- tests/vulpix-tests/meta/sbt-output.check | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/vulpix-tests/meta/sbt-output.check b/tests/vulpix-tests/meta/sbt-output.check index f8704de3ab6f..4d2a6f1f8293 100644 --- a/tests/vulpix-tests/meta/sbt-output.check +++ b/tests/vulpix-tests/meta/sbt-output.check @@ -8,7 +8,8 @@ Diff (expected on the left, actual right): EOF | EOF [error] Test dotty.tools.vulpix.VulpixMetaTests.runAll failed: java.lang.AssertionError: Run test failed, but should not, reasons: -[error] , took 1.682 sec SKIP +[error] +[error] - encountered 1 test failures(s), took 3.697 sec SKIP [error] at dotty.tools.vulpix.ParallelTesting$CompilationTest.checkRuns(ParallelTesting.scala:993) SKIP [error] at dotty.tools.vulpix.VulpixMetaTests.runAll(VulpixMetaTests.scala:26) SKIP [error] ... @@ -18,7 +19,7 @@ Testing tests/vulpix-tests/meta/neg/missing-error-annotation.scala Wrong number of errors encountered when compiling out/VulpixMetaTests/neg/missing-error-annotation, expected: 0, actual: 2 [error] Test dotty.tools.vulpix.VulpixMetaTests.compileNeg failed: java.lang.AssertionError: Neg test shouldn't have failed, but did. Reasons: [error] -[error] - encountered 2 error(s), took 0.093 sec SKIP +[error] - encountered 1 test failures(s), took 0.156 sec SKIP [error] at dotty.tools.vulpix.ParallelTesting$CompilationTest.checkExpectedErrors(ParallelTesting.scala:975) SKIP [error] at dotty.tools.vulpix.VulpixMetaTests.compileNeg(VulpixMetaTests.scala:25) SKIP [error] ... @@ -30,7 +31,7 @@ Testing tests/vulpix-tests/meta/pos/does-not-compile.scala | not found: a [error] Test dotty.tools.vulpix.VulpixMetaTests.compilePos failed: java.lang.AssertionError: Expected no errors when compiling, failed for the following reason(s): [error] -[error] - encountered 1 error(s), took 0.069 sec SKIP +[error] - encountered 1 test failure(s), took 0.069 sec SKIP [error] at dotty.tools.vulpix.ParallelTesting$CompilationTest.checkCompile(ParallelTesting.scala:958) SKIP [error] at dotty.tools.vulpix.VulpixMetaTests.compilePos(VulpixMetaTests.scala:24) SKIP [error] ... From 327340a18b7f9cecb7d6070c08bfe47d85475b9f Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 25 Apr 2018 23:04:36 +0200 Subject: [PATCH 5/8] VulpixUnitTests: Fix argument order for assertEquals This small issue swaps labels in the error output and complicated fixing failures. --- compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala b/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala index 1152a15aa594..ca7d36a2bdb6 100644 --- a/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala +++ b/compiler/test/dotty/tools/vulpix/VulpixUnitTests.scala @@ -91,7 +91,8 @@ class VulpixUnitTests extends ParallelTesting { fail() } catch { case ae: AssertionError => - assertEquals(ae.getMessage, "Run test failed, but should not, reasons:\n\n - encountered 1 test failures(s) - test 'tests/vulpix-tests/unit/timeout.scala' timed out") + assertEquals("Run test failed, but should not, reasons:\n\n - encountered 1 test failures(s) - test 'tests/vulpix-tests/unit/timeout.scala' timed out", + ae.getMessage) } } } From 4a98aa00ba6de17aa99733f90f34bb5380ef1f4c Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Fri, 27 Apr 2018 21:21:36 +0200 Subject: [PATCH 6/8] Cleanup a call --- compiler/test/dotty/tools/vulpix/ParallelTesting.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index b744f8227303..d08eaeb088ed 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -744,9 +744,7 @@ trait ParallelTesting extends RunnerOrchestration { self => true } else { - echo { - s"Error reported in ${error.pos.source}, but no annotation found" - } + echo(s"Error reported in ${error.pos.source}, but no annotation found") false } } From 006188e7f1c403ce13e590209c0123acf2ee8418 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Fri, 27 Apr 2018 22:15:42 +0200 Subject: [PATCH 7/8] Count multiple test failures correctly We can't use `_failures.size` since it's a Set. Using a `Seq` for now would be pointless, since most elements are just `Generic` failures, but would have a point if we wanted to show info on those failures in the final log. --- compiler/test/dotty/tools/vulpix/ParallelTesting.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index d08eaeb088ed..27a40306ecd5 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -240,17 +240,20 @@ trait ParallelTesting extends RunnerOrchestration { self => case object Generic extends Failure private[this] var _failures = Set.empty[Failure] + private[this] var _failureCount = 0 + /** Fail the current test */ protected[this] final def fail(failure: Failure = Generic): Unit = synchronized { _failures = _failures + failure + _failureCount = _failureCount + 1 } - def didFail: Boolean = _failures.nonEmpty + def didFail: Boolean = _failureCount != 0 /** A set of the different failures */ def failureReasons: Set[Failure] = _failures /** Number of failed tests */ - def failureCount: Int = _failures.size + def failureCount: Int = _failureCount protected def logBuildInstructions(reporter: TestReporter, testSource: TestSource, err: Int, war: Int) = { val errorMsg = testSource.buildInstructions(reporter.errorCount, reporter.warningCount) From 4300c075a2263e40ba36973373750d0b3c40c7bf Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Fri, 27 Apr 2018 22:20:52 +0200 Subject: [PATCH 8/8] Correct test report output This output refers to the number of CompilationTest instances with failures. --- compiler/test/dotty/tools/vulpix/SummaryReport.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/vulpix/SummaryReport.scala b/compiler/test/dotty/tools/vulpix/SummaryReport.scala index ee323782f29a..6a1988e58a82 100644 --- a/compiler/test/dotty/tools/vulpix/SummaryReport.scala +++ b/compiler/test/dotty/tools/vulpix/SummaryReport.scala @@ -98,7 +98,7 @@ final class SummaryReport extends SummaryReporting { |Test Report |================================================================================ | - |$passed passed, $failed failed, ${passed + failed} total + |$passed suites passed, $failed failed, ${passed + failed} total |""".stripMargin )