nspec / NSpec

A battle hardened testing framework for C# that's heavily inspired by Mocha and RSpec.
http://nspec.org/
MIT License
260 stars 57 forks source link

Execution Order before_all does not get called #190

Closed serialbandicoot closed 7 years ago

serialbandicoot commented 7 years ago

I've followed the execution order page: https://github.com/nspec/NSpec/wiki/Execution-Orders

However it does not seem to call before_all, my understanding is before_all should get called as it's included in the inherited parent?

I'm on version 3.0.7

using NSpec;
using System;

namespace NSpecExecutionOrder
{
    class parent_class : nspec 
    {

        void before_all()
        {
            throw new Exception("!!");
        }

    }

    class child_class : parent_class
    {
        void describe_test()
        {
            it["Should Fail"] = () =>
            {
                Console.WriteLine("Never make it here!");
            };

        }

    }
}

@amirrajan Congrats again for RubyMotion 👍

amirrajan commented 7 years ago

Hmm.. that's definitely a bug if from the looks of it. @BrainCrumbz something you can verify? If not, I'll take a look at Friday for sure.

(Thank you for the RM congrats. I appreciate it 😄 )

BrainCrumbz commented 7 years ago

Will have to look at this plus some other recent issues

BrainCrumbz commented 7 years ago

The new tests pass: they basically check that the right type of exception (both the internal NSpec one and the client code one) are correctly set.

But then we tried creating a small sample test library with the spec classes above, and actually they still pass, while they should not. Also, while debugging test run in VS, breakpoint set on parent before_all was not hit. [that was just VS test explorer playing games with us]

So there's something strange going on, will keep on working at this and keep you posted.

amirrajan commented 7 years ago

@serialbandicoot could you try this version of NSpec and see if the problem goes away? https://www.nuget.org/packages/nspec/1.0.13

BrainCrumbz commented 7 years ago

Small sample used:

class describe_parent_before_all_throwing : nspec
{
    void before_all()
    {
        Console.WriteLine("Parent before_all");
        throw new Exception("Parent before_all");
    }

    /*
    void before_each()
    {
        Console.WriteLine("Parent before_each");
        throw new Exception("Parent before_each");
    }
    */
}

class describe_child_before_all : describe_parent_before_all_throwing
{
    void describe_test()
    {
        it["Should Fail"] = () =>
        {
            Console.WriteLine("Never make it here!");
            true.ShouldBeTrue();
        };
    }
}

Dotnet core console runner output:

dotnet-test-nspec version: 0.2.1.0
NSpec version: 3.0.7.0
describe parent before all throwing
//Console output
Parent before_all
  describe child before all
    describe test
      Should Fail (8ms)
        //Console output
        Never make it here!
**** FAILURES ****
nspec. describe parent before all throwing. describe child before all. describe test. Should Fail.
Context Failure: Parent before_all
   at LibrarySpecs.describe_parent_before_all_throwing.before_all() in D:\WS\NET\NSpec-framework\NSpec\examples\DotNetTestSample\test\LibrarySpecs\d
escribe_parent_before_all_throwing.cs:line 13
1 Examples, 1 Failed, 0 Pending

VS2015 Test Explorer, still on .NET Core:

dotnettestsample - microsoft visual studio

Some thoughts:

Things don't change when replacing before_all with before_each.

BrainCrumbz commented 7 years ago

Same sample specs, on .NET Frramework. Output from NSpecRunner.exe:

> ..\..\packages\NSpec.3.0.7\tools\net451\win7-x64\NSpecRunner.exe bin\Debug\LibrarySpecs.dll
describe parent before all throwing
//Console output
Parent before_all
  describe child before all
    describe test
      Should Fail (78ms)
        //Console output
        Never make it here!
**** FAILURES ****
nspec. describe parent before all throwing. describe child before all. describe test. Should Fail.
Context Failure: Parent before_all
   at LibrarySpecs.describe_parent_before_all_throwing.before_all() in D:\WS\NET\NSpec-framework\NSpec\examples\NetFrameworkSample\test\LibrarySpecs
\describe_parent_before_all_throwing.cs:line 13
1 Examples, 1 Failed, 0 Pending

That behavior seems the same as dotnet core console runner. But the colored output from NSpecRunner gives more info:

cmder

Single spec is green, whole outcome is red. This needs to be looked at, definitely.

BrainCrumbz commented 7 years ago

Same sample specs, on .NET Framework but this time the test project is referencing NSpec 1.0.13. Maybe @serialbandicoot will confirm that too:

cmder_2

Referencing 0.9.68:

0 9 68


It seems the same to me, still something to be addressed though.

BrainCrumbz commented 7 years ago

Same output from NSpecRunner.exe also when referencing 1.0.7

BrainCrumbz commented 7 years ago

@amirrajan what should be the correct behaviour?

  1. For once, that's easy, single test case should be flagged as a failure.

  2. Should also not execute Example at all, because there's a previous "step" that has thrown? I was looking at Context.Exercise(...) implementation, to check if an example is supposed to run when e.g. a more "regular" before hooks has thrown.

amirrajan commented 7 years ago

For once, that's easy, single test case should be flagged as a failure.

Agreed.

Should also not execute Example at all, because there's a previous "step" that has thrown?

Probably not (deferring to RSpec's behavior):

describe 'before all' do
  before :all do
    @a = 1
    raise 'exception in before all'
  end

  it "doesn't run" do
    puts 'here'
    expect(@a + 1).to eq(2)
  end
end
Failures:

  1) before all doesn't run
     Failure/Error: raise 'exception in before all'

     RuntimeError:
       exception in before all
     # ./spec/sandbox_spec.rb:6:in `block (2 levels) in <top (required)>'
BrainCrumbz commented 7 years ago

Ok, so we have to requirements, when there's a failing, method-level before_all:

  1. Single test cases should be flagged as failure.

  2. Single test cases should not run at all.

Now it's time to see why existing NSpec tests are not catching those!

BrainCrumbz commented 7 years ago

@amirrajan maybe this is for a separate issue, but what's RSpec behavior in case of throwing before/beforeEach? Imagine again examples at same level, and nested ones, will not run at all and fail?

serialbandicoot commented 7 years ago

I'm back in the office (UK time) tomorrow, so I'll be able to look over any checks you need, if you keep me posted and I'll update the issue.

BrainCrumbz commented 7 years ago

Currently looking at a solution now, posting a fix branch soon. EDIT See fix/issue-190-before-all-throw and generated PR #192

BrainCrumbz commented 7 years ago

@serialbandicoot just to mention that PR #192 has been merged recently and it aims solving this issue plus other inconsistencies still related to exceptions thrown in beforeAll/before/act/...