From 832668ff6c2d12d575c2e47cbaf53cd71fa0a2a2 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Fri, 26 Apr 2019 12:15:19 +0000 Subject: [PATCH] Reland "[Infra] Let run_vm_tests --list output a test expectation marker" Due to not having support for __VA_OPT__ yet the CL introduces a new wet of macros ..._WITH_EXPECTATIONS() which can be given an expectation marker. Change-Id: I33812937f1b226fa89b3ab17a8a3483914abf2e4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100643 Reviewed-by: Stevie Strickland Commit-Queue: Martin Kustermann --- runtime/bin/run_vm_tests.cc | 4 +- runtime/tests/vm/vm.status | 9 --- .../compiler/assembler/assembler_ia32_test.cc | 8 +-- .../compiler/assembler/assembler_x64_test.cc | 8 +-- runtime/vm/object_test.cc | 6 +- runtime/vm/os_test.cc | 2 +- runtime/vm/unit_test.cc | 7 ++- runtime/vm/unit_test.h | 44 ++++++++++---- runtime/vm/zone_test.cc | 2 +- tools/testing/dart/test_runner.dart | 9 ++- tools/testing/dart/test_suite.dart | 59 +++++++++++++++---- 11 files changed, 107 insertions(+), 51 deletions(-) diff --git a/runtime/bin/run_vm_tests.cc b/runtime/bin/run_vm_tests.cc index 7fbad407759..70022e8b498 100644 --- a/runtime/bin/run_vm_tests.cc +++ b/runtime/bin/run_vm_tests.cc @@ -69,7 +69,7 @@ void TestCaseBase::RunTest() { this->Run(); run_matches++; } else if (run_filter == kList) { - Syslog::Print("%s\n", this->name()); + Syslog::Print("%s %s\n", this->name(), this->expectation()); run_matches++; } } @@ -82,7 +82,7 @@ void Benchmark::RunBenchmark() { this->score()); run_matches++; } else if (run_filter == kList) { - Syslog::Print("%s\n", this->name()); + Syslog::Print("%s Pass\n", this->name()); run_matches++; } } diff --git a/runtime/tests/vm/vm.status b/runtime/tests/vm/vm.status index 195b548fb91..0061e97940a 100644 --- a/runtime/tests/vm/vm.status +++ b/runtime/tests/vm/vm.status @@ -2,10 +2,6 @@ # for details. All rights reserved. Use of this source code is governed by a # BSD-style license that can be found in the LICENSE file. -cc/AllocGeneric_Overflow: Crash, Fail # These tests are expected to crash on all platforms. -cc/ArrayNew_Overflow_Crash: Crash, Fail # These tests are expected to crash on all platforms. -cc/CodeExecutability: Crash, Fail # These tests are expected to crash on all platforms. -cc/CodeImmutability: Crash, Fail # These tests are expected to crash on all platforms. cc/Dart2JSCompileAll: Fail, Crash # Issue 27369 cc/Dart2JSCompilerStats: Fail, Crash # Issue 27369 cc/Fail0: Fail # These tests are expected to crash on all platforms. @@ -18,7 +14,6 @@ cc/IsolateReload_PendingStaticCall_NSMToDefined: Fail, Crash # Issue 32981. Fail cc/IsolateReload_PendingUnqualifiedCall_InstanceToStatic: Fail # Issue 32981 cc/IsolateReload_PendingUnqualifiedCall_StaticToInstance: Fail # Issue 32981 cc/IsolateReload_RunNewFieldInitializersWithGenerics: Fail # Issue 32299 -cc/SNPrint_BadArgs: Crash, Fail # These tests are expected to crash on all platforms. dart/data_uri_import_test/none: SkipByDesign dart/snapshot_version_test: Skip # This test is a Dart1 test (script snapshot) dart/slow_path_shared_stub_test: Pass, Slow # Uses --shared-slow-path-triggers-gc flag. @@ -28,10 +23,6 @@ dart/use_bare_instructions_flag_test: Pass, Slow # Spawns several subprocesses [ $mode == debug ] dart/appjit_cha_deopt_test: Pass, Slow # Quite slow in debug mode, uses --optimization-counter-threshold=100 -[ $builder_tag == asan ] -cc/CodeExecutability: Fail, OK # Address Sanitizer turns a crash into a failure. -cc/CodeImmutability: Fail, OK # Address Sanitizer turns a crash into a failure. - [ $builder_tag == optimization_counter_threshold ] dart/appjit*: SkipByDesign # Test needs to a particular opt-counter value dart/kernel_determinism_test: SkipSlow diff --git a/runtime/vm/compiler/assembler/assembler_ia32_test.cc b/runtime/vm/compiler/assembler/assembler_ia32_test.cc index 26d37847513..1ba6a08fa8d 100644 --- a/runtime/vm/compiler/assembler/assembler_ia32_test.cc +++ b/runtime/vm/compiler/assembler/assembler_ia32_test.cc @@ -480,14 +480,14 @@ ASSEMBLER_TEST_RUN(Negate, test) { "ret\n"); } -ASSEMBLER_TEST_GENERATE(BitScanReverse, assembler) { +ASSEMBLER_TEST_GENERATE(BitScanReverseTest, assembler) { __ movl(ECX, Address(ESP, target::kWordSize)); __ movl(EAX, Immediate(666)); // Marker for conditional write. __ bsrl(EAX, ECX); __ ret(); } -ASSEMBLER_TEST_RUN(BitScanReverse, test) { +ASSEMBLER_TEST_RUN(BitScanReverseTest, test) { typedef int (*Bsr)(int input); Bsr call = reinterpret_cast(test->entry()); EXPECT_EQ(666, call(0)); @@ -4669,7 +4669,7 @@ ASSEMBLER_TEST_GENERATE(StoreIntoObject, assembler) { __ ret(); } -ASSEMBLER_TEST_GENERATE(BitTest, assembler) { +ASSEMBLER_TEST_GENERATE(BitTestTest, assembler) { __ movl(EAX, Immediate(4)); __ movl(ECX, Immediate(2)); __ bt(EAX, ECX); @@ -4681,7 +4681,7 @@ ASSEMBLER_TEST_GENERATE(BitTest, assembler) { __ ret(); } -ASSEMBLER_TEST_RUN(BitTest, test) { +ASSEMBLER_TEST_RUN(BitTestTest, test) { typedef int (*BitTest)(); EXPECT_EQ(1, reinterpret_cast(test->entry())()); EXPECT_DISASSEMBLY( diff --git a/runtime/vm/compiler/assembler/assembler_x64_test.cc b/runtime/vm/compiler/assembler/assembler_x64_test.cc index 9e380a46990..e8d688366ba 100644 --- a/runtime/vm/compiler/assembler/assembler_x64_test.cc +++ b/runtime/vm/compiler/assembler/assembler_x64_test.cc @@ -1085,7 +1085,7 @@ ASSEMBLER_TEST_RUN(Negate, test) { "ret\n"); } -ASSEMBLER_TEST_GENERATE(BitScanReverse, assembler) { +ASSEMBLER_TEST_GENERATE(BitScanReverseTest, assembler) { __ pushq(CallingConventions::kArg1Reg); __ movq(RCX, Address(RSP, 0)); __ movq(RAX, Immediate(666)); // Marker for conditional write. @@ -1094,7 +1094,7 @@ ASSEMBLER_TEST_GENERATE(BitScanReverse, assembler) { __ ret(); } -ASSEMBLER_TEST_RUN(BitScanReverse, test) { +ASSEMBLER_TEST_RUN(BitScanReverseTest, test) { typedef int (*Bsr)(int input); Bsr call = reinterpret_cast(test->entry()); EXPECT_EQ(666, call(0)); @@ -5484,7 +5484,7 @@ ASSEMBLER_TEST_RUN(ConditionalMovesCompare, test) { "ret\n"); } -ASSEMBLER_TEST_GENERATE(BitTest, assembler) { +ASSEMBLER_TEST_GENERATE(BitTestTest, assembler) { __ movq(RAX, Immediate(4)); __ movq(R11, Immediate(2)); __ btq(RAX, R11); @@ -5496,7 +5496,7 @@ ASSEMBLER_TEST_GENERATE(BitTest, assembler) { __ ret(); } -ASSEMBLER_TEST_RUN(BitTest, test) { +ASSEMBLER_TEST_RUN(BitTestTest, test) { typedef int (*BitTest)(); EXPECT_EQ(1, reinterpret_cast(test->entry())()); EXPECT_DISASSEMBLY( diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc index 4664ee7ae9a..ace33b6d1d7 100644 --- a/runtime/vm/object_test.cc +++ b/runtime/vm/object_test.cc @@ -2488,7 +2488,7 @@ ISOLATE_UNIT_TEST_CASE(Code) { // Test for immutability of generated instructions. The test crashes with a // segmentation fault when writing into it. -ISOLATE_UNIT_TEST_CASE(CodeImmutability) { +ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(CodeImmutability, "Crash") { bool stack_trace_collection_enabled = MallocHooks::stack_trace_collection_enabled(); MallocHooks::set_stack_trace_collection_enabled(false); @@ -2525,7 +2525,7 @@ class CodeTestHelper { // Test for executability of generated instructions. The test crashes with a // segmentation fault when executing the writeable view. -ISOLATE_UNIT_TEST_CASE(CodeExecutability) { +ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(CodeExecutability, "Crash") { bool stack_trace_collection_enabled = MallocHooks::stack_trace_collection_enabled(); MallocHooks::set_stack_trace_collection_enabled(false); @@ -3113,7 +3113,7 @@ ISOLATE_UNIT_TEST_CASE(EqualsIgnoringPrivate) { !String::EqualsIgnoringPrivateKey(ext_mangled_name, ext_bad_bare_name)); } -ISOLATE_UNIT_TEST_CASE(ArrayNew_Overflow_Crash) { +ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(ArrayNew_Overflow_Crash, "Crash") { Array::Handle(Array::New(Array::kMaxElements + 1)); } diff --git a/runtime/vm/os_test.cc b/runtime/vm/os_test.cc index 07b3cd961e2..406edf6209e 100644 --- a/runtime/vm/os_test.cc +++ b/runtime/vm/os_test.cc @@ -27,7 +27,7 @@ VM_UNIT_TEST_CASE(SNPrint) { } // This test is expected to crash when it runs. -VM_UNIT_TEST_CASE(SNPrint_BadArgs) { +VM_UNIT_TEST_CASE_WITH_EXPECTATION(SNPrint_BadArgs, "Crash") { int width = kMaxInt32; int num = 7; Utils::SNPrint(NULL, 0, "%*d%*d", width, num, width, num); diff --git a/runtime/vm/unit_test.cc b/runtime/vm/unit_test.cc index 23b42eead63..398ecc8e647 100644 --- a/runtime/vm/unit_test.cc +++ b/runtime/vm/unit_test.cc @@ -52,8 +52,11 @@ TestCaseBase* TestCaseBase::first_ = NULL; TestCaseBase* TestCaseBase::tail_ = NULL; KernelBufferList* TestCaseBase::current_kernel_buffers_ = NULL; -TestCaseBase::TestCaseBase(const char* name) - : raw_test_(false), next_(NULL), name_(name) { +TestCaseBase::TestCaseBase(const char* name, const char* expectation) + : raw_test_(false), + next_(NULL), + name_(name), + expectation_(strlen(expectation) > 0 ? expectation : "Pass") { if (first_ == NULL) { first_ = this; } else { diff --git a/runtime/vm/unit_test.h b/runtime/vm/unit_test.h index c4855d2c200..bbdfea4adc1 100644 --- a/runtime/vm/unit_test.h +++ b/runtime/vm/unit_test.h @@ -23,27 +23,33 @@ // The VM_UNIT_TEST_CASE macro is used for tests that do not need any // default isolate or zone functionality. -#define VM_UNIT_TEST_CASE(name) \ +#define VM_UNIT_TEST_CASE_WITH_EXPECTATION(name, expectation) \ void Dart_Test##name(); \ - static const dart::TestCase kRegister##name(Dart_Test##name, #name); \ + static const dart::TestCase kRegister##name(Dart_Test##name, #name, \ + expectation); \ void Dart_Test##name() +#define VM_UNIT_TEST_CASE(name) VM_UNIT_TEST_CASE_WITH_EXPECTATION(name, "Pass") + // The UNIT_TEST_CASE macro is used for tests that do not require any // functionality provided by the VM. Tests declared using this macro will be run // after the VM is cleaned up. -#define UNIT_TEST_CASE(name) \ +#define UNIT_TEST_CASE_WITH_EXPECTATION(name, expectation) \ void Dart_Test##name(); \ - static const dart::RawTestCase kRegister##name(Dart_Test##name, #name); \ + static const dart::RawTestCase kRegister##name(Dart_Test##name, #name, \ + expectation); \ void Dart_Test##name() +#define UNIT_TEST_CASE(name) UNIT_TEST_CASE_WITH_EXPECTATION(name, "Pass") + // The ISOLATE_UNIT_TEST_CASE macro is used for tests that need an isolate and // zone in order to test its functionality. This macro is used for tests that // are implemented using the VM code directly and do not use the Dart API // for calling into the VM. The safepoint execution state of threads using // this macro is transitioned from kThreadInNative to kThreadInVM. -#define ISOLATE_UNIT_TEST_CASE(name) \ +#define ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(name, expectation) \ static void Dart_TestHelper##name(Thread* thread); \ - VM_UNIT_TEST_CASE(name) { \ + VM_UNIT_TEST_CASE_WITH_EXPECTATION(name, expectation) { \ TestIsolateScope __test_isolate__; \ Thread* __thread__ = Thread::Current(); \ ASSERT(__thread__->isolate() == __test_isolate__.isolate()); \ @@ -54,13 +60,16 @@ } \ static void Dart_TestHelper##name(Thread* thread) +#define ISOLATE_UNIT_TEST_CASE(name) \ + ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(name, "Pass") + // The TEST_CASE macro is used for tests that need an isolate and zone // in order to test its functionality. This macro is used for tests that // are implemented using the Dart API for calling into the VM. The safepoint // execution state of threads using this macro remains kThreadNative. -#define TEST_CASE(name) \ +#define TEST_CASE_WITH_EXPECTATION(name, expectation) \ static void Dart_TestHelper##name(Thread* thread); \ - VM_UNIT_TEST_CASE(name) { \ + VM_UNIT_TEST_CASE_WITH_EXPECTATION(name, expectation) { \ TestIsolateScope __test_isolate__; \ Thread* __thread__ = Thread::Current(); \ ASSERT(__thread__->isolate() == __test_isolate__.isolate()); \ @@ -72,6 +81,8 @@ } \ static void Dart_TestHelper##name(Thread* thread) +#define TEST_CASE(name) TEST_CASE_WITH_EXPECTATION(name, "Pass") + // The ASSEMBLER_TEST_GENERATE macro is used to generate a unit test // for the assembler. #define ASSEMBLER_TEST_GENERATE(name, assembler) \ @@ -85,9 +96,9 @@ // The ASSEMBLER_TEST_RUN macro is used to execute the assembler unit // test generated using the ASSEMBLER_TEST_GENERATE macro. // C++ callee-saved registers are not preserved. Arguments may be passed in. -#define ASSEMBLER_TEST_RUN(name, test) \ +#define ASSEMBLER_TEST_RUN_WITH_EXPECTATION(name, test, expectation) \ static void AssemblerTestRun##name(AssemblerTest* test); \ - ISOLATE_UNIT_TEST_CASE(name) { \ + ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(name, expectation) { \ { \ bool use_far_branches = false; \ LongJumpScope jump; \ @@ -117,6 +128,9 @@ } \ static void AssemblerTestRun##name(AssemblerTest* test) +#define ASSEMBLER_TEST_RUN(name, test) \ + ASSEMBLER_TEST_RUN_WITH_EXPECTATION(name, test, "Pass") + #if defined(TARGET_ARCH_ARM) || defined(TARGET_ARCH_ARM64) #if defined(HOST_ARCH_ARM) || defined(HOST_ARCH_ARM64) // Running on actual ARM hardware, execute code natively. @@ -249,10 +263,11 @@ class KernelBufferList { class TestCaseBase { public: - explicit TestCaseBase(const char* name); + explicit TestCaseBase(const char* name, const char* expectation); virtual ~TestCaseBase() {} const char* name() const { return name_; } + const char* expectation() const { return expectation_; } virtual void Run() = 0; void RunTest(); @@ -272,6 +287,7 @@ class TestCaseBase { TestCaseBase* next_; const char* name_; + const char* expectation_; DISALLOW_COPY_AND_ASSIGN(TestCaseBase); }; @@ -284,7 +300,8 @@ class TestCase : TestCaseBase { public: typedef void(RunEntry)(); - TestCase(RunEntry* run, const char* name) : TestCaseBase(name), run_(run) {} + TestCase(RunEntry* run, const char* name, const char* expectation) + : TestCaseBase(name, expectation), run_(run) {} static char* CompileTestScriptWithDFE(const char* url, const char* source, @@ -391,7 +408,8 @@ class RawTestCase : TestCaseBase { public: typedef void(RunEntry)(); - RawTestCase(RunEntry* run, const char* name) : TestCaseBase(name), run_(run) { + RawTestCase(RunEntry* run, const char* name, const char* expectation) + : TestCaseBase(name, expectation), run_(run) { raw_test_ = true; } virtual void Run(); diff --git a/runtime/vm/zone_test.cc b/runtime/vm/zone_test.cc index 5f8f78305e7..ff9f6b5b739 100644 --- a/runtime/vm/zone_test.cc +++ b/runtime/vm/zone_test.cc @@ -92,7 +92,7 @@ VM_UNIT_TEST_CASE(AllocGeneric_Success) { } // This test is expected to crash. -VM_UNIT_TEST_CASE(AllocGeneric_Overflow) { +VM_UNIT_TEST_CASE_WITH_EXPECTATION(AllocGeneric_Overflow, "Crash") { #if defined(DEBUG) FLAG_trace_zones = true; #endif diff --git a/tools/testing/dart/test_runner.dart b/tools/testing/dart/test_runner.dart index 6deb34adbd9..ed69296521f 100644 --- a/tools/testing/dart/test_runner.dart +++ b/tools/testing/dart/test_runner.dart @@ -80,6 +80,7 @@ class TestCase extends UniqueObject { static final int HAS_SYNTAX_ERROR = 1 << 1; static final int HAS_COMPILE_ERROR = 1 << 2; static final int HAS_STATIC_WARNING = 1 << 3; + static final int HAS_CRASH = 1 << 4; /** * A list of commands to execute. Most test cases have a single command. * Dart2js tests have two commands, one to compile the source and another @@ -104,7 +105,8 @@ class TestCase extends UniqueObject { if (info != null) { _setExpectations(info); - hash = info.originTestPath.relativeTo(Repository.dir).toString().hashCode; + hash = (info?.originTestPath?.relativeTo(Repository.dir)?.toString()) + .hashCode; } } @@ -113,6 +115,7 @@ class TestCase extends UniqueObject { // so we copy the needed bools into flags set in a single integer. if (info.hasRuntimeError) _expectations |= HAS_RUNTIME_ERROR; if (info.hasSyntaxError) _expectations |= HAS_SYNTAX_ERROR; + if (info.hasCrash) _expectations |= HAS_CRASH; if (info.hasCompileError || info.hasSyntaxError) { _expectations |= HAS_COMPILE_ERROR; } @@ -131,6 +134,7 @@ class TestCase extends UniqueObject { bool get hasStaticWarning => _expectations & HAS_STATIC_WARNING != 0; bool get hasSyntaxError => _expectations & HAS_SYNTAX_ERROR != 0; bool get hasCompileError => _expectations & HAS_COMPILE_ERROR != 0; + bool get hasCrash => _expectations & HAS_CRASH != 0; bool get isNegative => hasCompileError || hasRuntimeError && configuration.runtime != Runtime.none || @@ -146,6 +150,9 @@ class TestCase extends UniqueObject { Expectation get result => lastCommandOutput.result(this); Expectation get realResult => lastCommandOutput.realResult(this); Expectation get realExpected { + if (hasCrash) { + return Expectation.crash; + } if (configuration.compiler == Compiler.specParser) { if (hasSyntaxError) { return Expectation.syntaxError; diff --git a/tools/testing/dart/test_suite.dart b/tools/testing/dart/test_suite.dart index b3ad21dd2e1..620245507a0 100644 --- a/tools/testing/dart/test_suite.dart +++ b/tools/testing/dart/test_suite.dart @@ -451,22 +451,46 @@ class VMTestSuite extends TestSuite { var expectations = new ExpectationSet.read(statusFiles, configuration); try { - for (var name in await _listTests(hostRunnerPath)) { - _addTest(expectations, name); + for (VmUnitTest test in await _listTests(hostRunnerPath)) { + _addTest(expectations, test); } doTest = null; if (onDone != null) onDone(); - } catch (error) { + } catch (error, s) { print("Fatal error occured: $error"); + print(s); exit(1); } } - void _addTest(ExpectationSet testExpectations, String testName) { - var fullName = 'cc/$testName'; + void _addTest(ExpectationSet testExpectations, VmUnitTest test) { + final fullName = 'cc/${test.name}'; var expectations = testExpectations.expectations(fullName); + // Get the expectation from the cc/ test itself. + final Expectation testExpectation = Expectation.find(test.expectation); + + // Update the legacy status-file based expectations to include + // [testExpectation]. + if (testExpectation != Expectation.pass) { + expectations = Set.from(expectations)..add(testExpectation); + expectations.removeWhere((e) => e == Expectation.pass); + } + + // Update the new workflow based expectations to include [testExpectation]. + final Path filePath = null; + final Path originTestPath = null; + final hasSyntaxError = false; + final hasStaticWarning = false; + final hasCompileTimeError = testExpectation == Expectation.compileTimeError; + final hasRuntimeError = testExpectation == Expectation.runtimeError; + final hasCrash = testExpectation == Expectation.crash; + final optionsFromFile = const {}; + final testInfo = TestInformation(filePath, originTestPath, optionsFromFile, + hasSyntaxError, hasCompileTimeError, hasRuntimeError, hasStaticWarning, + hasCrash: hasCrash); + var args = configuration.standardOptions.toList(); if (configuration.compilerConfiguration.previewDart2) { final filename = configuration.architecture == Architecture.x64 @@ -480,14 +504,14 @@ class VMTestSuite extends TestSuite { args.insert(0, '--suppress-core-dump'); } - args.add(testName); + args.add(test.name); - var command = Command.process( + final command = Command.process( 'run_vm_unittest', targetRunnerPath, args, environmentOverrides); - enqueueNewTestCase(fullName, [command], expectations); + enqueueNewTestCase(fullName, [command], expectations, testInfo); } - Future> _listTests(String runnerPath) async { + Future> _listTests(String runnerPath) async { var result = await Process.run(runnerPath, ["--list"]); if (result.exitCode != 0) { throw "Failed to list tests: '$runnerPath --list'. " @@ -497,10 +521,21 @@ class VMTestSuite extends TestSuite { return (result.stdout as String) .split('\n') .map((line) => line.trim()) - .where((name) => name.isNotEmpty); + .where((name) => name.isNotEmpty) + .map((String line) { + final parts = line.split(' '); + return VmUnitTest(parts[0].trim(), parts.skip(1).single); + }); } } +class VmUnitTest { + final String name; + final String expectation; + + VmUnitTest(this.name, this.expectation); +} + class TestInformation { Path filePath; Path originTestPath; @@ -509,6 +544,7 @@ class TestInformation { bool hasCompileError; bool hasRuntimeError; bool hasStaticWarning; + bool hasCrash; String multitestKey; TestInformation( @@ -519,7 +555,8 @@ class TestInformation { this.hasCompileError, this.hasRuntimeError, this.hasStaticWarning, - {this.multitestKey: ''}) { + {this.multitestKey: '', + this.hasCrash: false}) { assert(filePath.isAbsolute); } }