Added new type of unit test, RAW_UNIT_TEST_CASE, which is used for tests that can be flaky if run w…

Added new type of unit test, RAW_UNIT_TEST_CASE, which is used for tests that can be flaky if run while the VM is alive. Tests created using the RAW_UNIT_TEST_CASE macro will run after the VM has shutdown to avoid having any worker threads contaminating the test results.

BUG=
R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/f2fd150ea04bc33976ca2fc4bdbf15655eabe207

[bkonyi, 2017-01-31 20:59:49]

bkonyi@google.com changed reviewers:
+ johnmccutchan@google.com

[bkonyi, 2017-01-31 20:59:50]

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
File runtime/bin/run_vm_tests.cc (right):

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
runtime/bin/run_vm_tests.cc:133: TestCaseBase::RunAllRaw();
The raw tests are run after the VM is shutdown since the MallocHooks
implementation is using MutexLockers, which require that there is a current
thread. This is initialized in OSThread::InitOnce(), so we can't run those tests
until after the VM has been initialized. Since Dart::Cleanup() doesn't actually
cleanup anything in OSThread::Cleanup(), at this point there is still a current
thread but the VM has been shutdown. This feels like a bit of a hack, so someone
else might have a better solution.

[Cutch, 2017-01-31 21:35:53]

johnmccutchan@google.com changed reviewers:
+ rmacnak@google.com

[Cutch, 2017-01-31 21:35:53]

+Ryan Macnak

Based on discussions we had with Ryan I think the correct renaming is:

RAW_UNIT_TEST_CASE -> UNIT_TEST_CASE
UNIT_TEST_CASE -> VM_UNIT_TEST_CASE 
VM_UNIT_TEST_CASE -> ISOLATE_UNIT_TEST_CASE

[rmacnak, 2017-01-31 21:43:42]

+1 on macro renames

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
File runtime/bin/run_vm_tests.cc (right):

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
runtime/bin/run_vm_tests.cc:45: fprintf(stdout, "Running test: %s\n", name());
OS::Print

[bkonyi, 2017-01-31 22:14:35]

I've updated the macro names to the ones in an earlier comment.

Would it make more sense for me to change the RawUnitTest methods/classes to
something like NoVMUnitTest?

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
File runtime/bin/run_vm_tests.cc (right):

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
runtime/bin/run_vm_tests.cc:45: fprintf(stdout, "Running test: %s\n", name());
On 2017/01/31 21:43:42, rmacnak wrote:
> OS::Print

fprintf is used throughout run_vm_tests.cc. Should these other instances be
switched to use OS::Print or should I just stick with fprintf?

[rmacnak, 2017-01-31 22:44:39]

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
File runtime/bin/run_vm_tests.cc (right):

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
runtime/bin/run_vm_tests.cc:45: fprintf(stdout, "Running test: %s\n", name());
On 2017/01/31 22:14:35, bkonyi wrote:
> On 2017/01/31 21:43:42, rmacnak wrote:
> > OS::Print
> 
> fprintf is used throughout run_vm_tests.cc. Should these other instances be
> switched to use OS::Print or should I just stick with fprintf?

They ought to be OS::Print so one can actually read the output on Android/iOS,
but you don't need to update them in this CL.

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/malloc_hooks...
File runtime/vm/malloc_hooks_test.cc (right):

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/malloc_hooks...
runtime/vm/malloc_hooks_test.cc:28: RAW_VM_UNIT_TEST_CASE(BasicMallocHookTest) {
There's no VM :)
RAW_UNIT_TEST or UNIT_TEST_NO_VM, etc.

[Cutch, 2017-01-31 22:47:35]

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/unit_test.h
File runtime/vm/unit_test.h (right):

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/unit_test.h#...
runtime/vm/unit_test.h:34: #define RAW_VM_UNIT_TEST_CASE(name)                  
                         \
UNIT_TEST_CASE

[bkonyi, 2017-01-31 22:57:19]

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
File runtime/bin/run_vm_tests.cc (right):

https://codereview.chromium.org/2666133002/diff/40001/runtime/bin/run_vm_test...
runtime/bin/run_vm_tests.cc:45: fprintf(stdout, "Running test: %s\n", name());
On 2017/01/31 22:44:39, rmacnak wrote:
> On 2017/01/31 22:14:35, bkonyi wrote:
> > On 2017/01/31 21:43:42, rmacnak wrote:
> > > OS::Print
> > 
> > fprintf is used throughout run_vm_tests.cc. Should these other instances be
> > switched to use OS::Print or should I just stick with fprintf?
> 
> They ought to be OS::Print so one can actually read the output on Android/iOS,
> but you don't need to update them in this CL.

Ah okay. I'll update all of the fprintf instances (including this one) in
another CL then.

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/malloc_hooks...
File runtime/vm/malloc_hooks_test.cc (right):

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/malloc_hooks...
runtime/vm/malloc_hooks_test.cc:28: RAW_VM_UNIT_TEST_CASE(BasicMallocHookTest) {
On 2017/01/31 22:44:39, rmacnak wrote:
> There's no VM :)
> RAW_UNIT_TEST or UNIT_TEST_NO_VM, etc.

Oops, apparently my sed-foo needs some work... I've changed these to
UNIT_TEST_CASE.

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/unit_test.h
File runtime/vm/unit_test.h (right):

https://codereview.chromium.org/2666133002/diff/60001/runtime/vm/unit_test.h#...
runtime/vm/unit_test.h:34: #define RAW_VM_UNIT_TEST_CASE(name)                  
                         \
On 2017/01/31 22:47:35, Cutch wrote:
> UNIT_TEST_CASE

Done.

[Cutch, 2017-01-31 23:03:08]

lgtm

[bkonyi, 2017-01-31 23:16:45]

Description was changed from

==========
Added new type of unit test, RAW_UNIT_TEST_CASE, which is used for tests that
can be flaky if run while the VM is alive. Tests created using the
RAW_UNIT_TEST_CASE macro will run after the VM has shutdown to avoid having any
worker threads contaminating the test results.

BUG=
==========

to

==========
Added new type of unit test, RAW_UNIT_TEST_CASE, which is used for tests that
can be flaky if run while the VM is alive. Tests created using the
RAW_UNIT_TEST_CASE macro will run after the VM has shutdown to avoid having any
worker threads contaminating the test results.

BUG=
R=johnmccutchan@google.com

Committed:
https://github.com/dart-lang/sdk/commit/f2fd150ea04bc33976ca2fc4bdbf15655eabe207
==========

[bkonyi, 2017-01-31 23:16:46]

Committed patchset #5 (id:80001) manually as
f2fd150ea04bc33976ca2fc4bdbf15655eabe207 (presubmit successful).