Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execution Order before_all does not get called #190

Closed
serialbandicoot opened this issue May 3, 2017 · 15 comments
Closed

Execution Order before_all does not get called #190

serialbandicoot opened this issue May 3, 2017 · 15 comments

Comments

@serialbandicoot
Copy link

serialbandicoot commented May 3, 2017

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
Copy link
Collaborator

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
Copy link
Collaborator

Will have to look at this plus some other recent issues

@BrainCrumbz
Copy link
Collaborator

BrainCrumbz commented May 11, 2017

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
Copy link
Collaborator

@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
Copy link
Collaborator

BrainCrumbz commented May 11, 2017

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:

  • Dotnet core console runner reports failure, but still that test code is executed. Not sure if this is the way is supposed to be, will have to check

  • VS2015 (integrating with same dotnet core runner behind the scenes) does not report any failure at all. This is certainly an issue

Things don't change when replacing before_all with before_each.

@BrainCrumbz
Copy link
Collaborator

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
Copy link
Collaborator

BrainCrumbz commented May 11, 2017

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
Copy link
Collaborator

Same output from NSpecRunner.exe also when referencing 1.0.7

@BrainCrumbz
Copy link
Collaborator

@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
Copy link
Collaborator

amirrajan commented May 11, 2017

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Author

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
Copy link
Collaborator

BrainCrumbz commented May 11, 2017

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

@BrainCrumbz
Copy link
Collaborator

@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/...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants