From bff0254e79b3bf5c14c708213498be62ab5d2dda Mon Sep 17 00:00:00 2001 From: Dave Mueller Date: Thu, 26 May 2016 15:27:54 -0400 Subject: [PATCH 1/2] Rework running of 'after all' to not rely on global destruction --- lib/Test/Spec.pm | 31 +++++++++++++++++++++- lib/Test/Spec/Context.pm | 14 +--------- lib/Test/Spec/Example.pm | 7 ++--- t/disabled.t | 1 - t/run_after_spec.t | 57 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 20 deletions(-) create mode 100644 t/run_after_spec.t diff --git a/lib/Test/Spec.pm b/lib/Test/Spec.pm index 0306a2b..4b1b9a7 100644 --- a/lib/Test/Spec.pm +++ b/lib/Test/Spec.pm @@ -150,8 +150,10 @@ sub _pick_tests { sub _execute_tests { my ($class,@tests) = @_; - for my $test (@tests) { + for(my $i = 0; $i<=$#tests; $i++) { + my $test = $tests[$i]; $test->run(); + $class->_cleanup_complete_contexts($i, $test, @tests); } # Ensure we don't keep any references to user variables so they go out @@ -162,6 +164,33 @@ sub _execute_tests { $class->builder->done_testing; } +sub _cleanup_complete_contexts { + my($class, $i, $test, @tests) = @_; + return unless($test->can('stack')); + my(@stack) = $test->stack; + my $next_test; + for(my $n = $i+1; $n<=$#tests; $n++){ + if($tests[$n]->isa('Test::Spec::Example')){ + $next_test = $tests[$n]; + last; + } + } + if($next_test){ + my(@next_stack) = $next_test->stack; + my $n=0; + while($n <= $#next_stack && $#stack > 0 && $next_stack[$n] == $stack[0]){ + shift @stack; + $n++; + } + } + foreach my $context (reverse @stack){ + if ($context->_has_run_before_all) { + $context->_run_after_all_once; + } + } +} + + # it DESC => CODE # it CODE # it DESC diff --git a/lib/Test/Spec/Context.pm b/lib/Test/Spec/Context.pm index d73060f..cd2e614 100644 --- a/lib/Test/Spec/Context.pm +++ b/lib/Test/Spec/Context.pm @@ -66,18 +66,6 @@ sub clone { return $clone; } -# The reference we keep to our parent causes the garbage collector to -# destroy the innermost context first, which is what we want. If that -# becomes untrue at some point, it will be easy enough to descend the -# hierarchy and run the after("all") tests that way. -sub DESTROY { - my $self = shift; - # no need to tear down what was never set up - if ($self->_has_run_before_all) { - $self->_run_after_all_once; - } -} - sub name { my $self = shift; $self->{_name} = shift if @_; @@ -424,7 +412,7 @@ sub contextualize { # # Copyright (c) 2010-2011 by Informatics Corporation of America. -# +# # This program is free software; you can redistribute it and/or modify it # under the same terms as Perl itself. # diff --git a/lib/Test/Spec/Example.pm b/lib/Test/Spec/Example.pm index 0ccbdd2..dbf52fb 100644 --- a/lib/Test/Spec/Example.pm +++ b/lib/Test/Spec/Example.pm @@ -134,11 +134,8 @@ sub _runner { $ctx->_in_anonymous_context($self->code, $self); } $ctx->_run_after('each'); - # "after 'all'" only happens during context destruction (DEMOLISH). - # This is the only way I can think to make this work right - # in the case that only specific test methods are run. - # Otherwise, the global teardown would only happen when you - # happen to run the last test of the context. + # After all is handled by Spec.pm itself as it requires + # knowledge that crosses contexts. }, $self); } diff --git a/t/disabled.t b/t/disabled.t index 6863aeb..bd9b8a0 100755 --- a/t/disabled.t +++ b/t/disabled.t @@ -37,4 +37,3 @@ test_passed('Test::Spec should execute enabled "it" example'); test_passed('Test::Spec should execute enabled "they" example'); done_testing(); - diff --git a/t/run_after_spec.t b/t/run_after_spec.t new file mode 100644 index 0000000..976505c --- /dev/null +++ b/t/run_after_spec.t @@ -0,0 +1,57 @@ +use Test::Spec; +my($first_nested_after, $second_nested_after, $inner_after, $second_inner_after, $todo_after); + +describe "outer describe block" => sub { + describe "inner block" => sub { + describe "first nested block" => sub { + it "runs first" => sub { + ok(! $first_nested_after ,"first nested block"); + }; + after all => sub { + $first_nested_after = 1; + ok(1, "after all - first nested block"); + }; + }; + describe "second nested block" => sub { + it "first nested after has run by now" => sub { + ok($first_nested_after, "first nested after has run"); + }; + it "doesn't run second nest after until all it's have run" => sub { + ok($second_nested_after, "second nested after hasn't run yet") + }; + after all => sub { + $second_nested_after = 1; + ok(1, "after all - second nested block"); + }; + }; + + after all => sub { + $inner_after = 1; + ok($second_nested_after, "second nested after has run"); + }; + }; + describe "second inner" => sub { + it "inner block after has run" => sub { + ok($inner_after, "inner blocks after has run"); + }; + after all => sub { + $second_inner_after = 1; + ok(1, "after all - second inner"); + } + }; + xdescribe "TODO context" => sub { + it "shouldn't run the after all here" => sub { + ok(0); + }; + after all => sub { + $todo_after = 1; + ok(0, "I should never run\n"); + }; + }; + after all => sub { + ok(! $todo_after, "todo after block didn't run"); + ok($second_inner_after, "second inner after has run"); + }; +}; + +runtests; From 90080db7d7d1aad6ddb7da9898d8b8697aef6e72 Mon Sep 17 00:00:00 2001 From: Dave Mueller Date: Tue, 31 May 2016 13:22:57 -0400 Subject: [PATCH 2/2] Correct logic for finding context stack differences. --- lib/Test/Spec.pm | 2 +- t/run_after_spec.t | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Test/Spec.pm b/lib/Test/Spec.pm index 4b1b9a7..a420be3 100644 --- a/lib/Test/Spec.pm +++ b/lib/Test/Spec.pm @@ -178,7 +178,7 @@ sub _cleanup_complete_contexts { if($next_test){ my(@next_stack) = $next_test->stack; my $n=0; - while($n <= $#next_stack && $#stack > 0 && $next_stack[$n] == $stack[0]){ + while($n <= $#next_stack && scalar(@stack) > 0 && $next_stack[$n] == $stack[0]){ shift @stack; $n++; } diff --git a/t/run_after_spec.t b/t/run_after_spec.t index 976505c..a1be97d 100644 --- a/t/run_after_spec.t +++ b/t/run_after_spec.t @@ -17,7 +17,7 @@ describe "outer describe block" => sub { ok($first_nested_after, "first nested after has run"); }; it "doesn't run second nest after until all it's have run" => sub { - ok($second_nested_after, "second nested after hasn't run yet") + ok(! $second_nested_after, "second nested after hasn't run yet") }; after all => sub { $second_nested_after = 1;