Testing a custom bootstrapper

[alexsaare]

Hi, I raised this stackoverflow question

http://stackoverflow.com/questions/33830206/testing-code-in-a-custom-nancyfx-bootstrapper/3386149

For completeness I'll repeat the custom bootstrapper here

public class CustomNancyBootstrapper : StructureMapNancyBootstrapper
{
    protected override void RequestStartup(IContainer container, IPipelines pipelines, NancyContext context)
    {
        var auth = container.GetInstance<ICustomAuth>();
        auth.Authenticate(context);
    }
}  

I want to write a test to assert that Authenticate is called with the context.

The issue is the same regardless of container.

As far as i can see you are faced with either 2 options 1 - Derive another class from your own custom bootstrapper which has a public method which calls the protected method... not great but the test is fairly concise easy to read.

[Test]
public void RequestStartup_Calls_CustomAuth_Authenticate_WithContext()
{
    // set up
    var mockAuthentication = new Mock<ICustomAuth>();
    var mockContainer = new Mock<IContainer>();
    var mockPipelines = new Mock<IPipelines>();
    var context = new NancyContext();

    mockContainer.Setup(x => x.GetInstance<ICustomAuth>()).Returns(mockAuthentication.Object);

    var bootstrapper = new CustomNancyBootstrapperWrapper()

    // exercise
    bootstrapper.RequestStartupPublic(_mockContainer.Object, _mockPipelines.Object, context);

    // verify
    mockAuthentication.Verify(x => x.Authenticate(context), Times.Once);
}

private class CustomNancyBootstrapperWrapper : CustomNancyBootstrapper 
{
    public void RequestStartupPublic(IContainer container, IPipelines pipelines, NancyContext context)
    {
        RequestStartup(container, pipelines, context);
    }
}

2 - Use the Browser offered in Nancy.Testing however, this is really testing the route/module and only testing the bootstrapper indirectly. This is very difficult to read and it's very brittle...

    [Test]
    public void When_Route_Is_Requested_Bootstrapper_Calls_Authenticate_Via_RequestStartup()
    {
        var mockAuthentication = new Mock<ICustomAuth>();

        var container = ContainerWrapper.Instance;

        container.Configure(c =>
        {
            c.For<ICustomAuth>().Use(() => mockAuthentication.Object);
        });

        var browser = new Browser(new CustomNancyBootstrapper());

        // exercise
        browser.Get("/");

        // verify
        mockAuthentication.Verify(x => x.Authenticate(It.IsAny<NancyContext>()), Times.Once);
    }

Also, note.. there's no way to test that a specific NancyContext was used and you have to resort to

  It.IsAny<NancyContext>()

So If I change my bootstrapper to this...

public class CustomNancyBootstrapper : StructureMapNancyBootstrapper
{
    protected override void RequestStartup(IContainer container, IPipelines pipelines, NancyContext context)
    {
        var auth = container.GetInstance<ICustomAuth>();
        auth.Authenticate(new NancyContext());
    }
}  

My test still passes but my application wouldn't work... making my test pretty much pointless.

Any thoughts on testing a custom bootstrapper more explicitly would be appreciated. What is the benefit of these methods being protected? Given NancyBootStrapperBase is abstract, any derived types will contain code which should be tested. The protected modifiers really make that really difficult.

[khellang]

What is the benefit of these methods being protected?

These methods are protected because, surprise surprise, they're not meant to be called by outsiders. The Nancy bootstrapper holds state and has a specific ordering in which the methods should/needs to be called. If these are not called in the correct order, you might corrupt that state and things start to fail. This is a classic example of the Hollywood principle; "don't call us, we'll call you".

Why are you even trying to test that the RequestStartup method is being called? Are you trying to test that the framework is doing its job?

The second example, where you're using Nancy.Testing, would be the best way to achieve what you want, and IMO, that test is much easier to read than the one above. It makes little sense to unit test the internals of the bootstrapper in the way you want.

this is really testing the route/module and only testing the bootstrapper indirectly. This is very difficult to read and it's very brittle...

It tests exactly what you want it to test. If you set up your mock and verify it afterwards, why isn't that testing what you want? And how is it brittle?

Side note; where are you passing your container to the bootstrapper? I can't see the container variable being used...

there's no way to test that a specific NancyContext was used and you have to resort to It.IsAny<NancyContext>()

There's a couple of ways you can assert on the NancyContext being passed to the method;

1. You can supply your own INancyContextFactory, if you want to verify a specific instance.
2. You can use FakeItEasy to do the verification of the context, A<NancyContext>.That.Matches(ctx => { /* do matching here */ });

[khellang]

Oh, didn't realize you were using Moq (it's been a while :wink:).

With Moq, you can do It.Is<NancyContext>(ctx => { /* do matching here */ });.

[alexsaare]

Why are you even trying to test that the RequestStartup method is being called?

I'm not trying to test that RequestStartup is called, I'm trying to test my code inside it.

It makes little sense to unit test the internals of the bootstrapper in the way you want. The sample above is simplified, there could be plenty of code in there I should test. Again, I'm not testing the bootstrapper is called, I'm testing my code inside it.

If you set up your mock and verify it afterwards, why isn't that testing what you want? And how is it brittle?

You may have missed my edit but It's brittle because of this...

If I change my bootstrapper to this...

public class CustomNancyBootstrapper : StructureMapNancyBootstrapper
{
    protected override void RequestStartup(IContainer container, IPipelines pipelines, NancyContext context)
    {
        var auth = container.GetInstance<ICustomAuth>();
        auth.Authenticate(new NancyContext());
    }
}  

My test still passes but my application wouldn't work... making my test pretty much pointless.

also, the route specified might easily have an effect on the test. If someone else changes the module, my test could fail for no good reason. Calling the route does not explicitly mean I'm testing the code in RequestStartup... It's testing it by proxy.

These methods are protected because, surprise surprise, they're not meant to be called by outsiders. The Nancy bootstrapper holds state and has a specific ordering in which the methods should/needs to be called. If these are not called in the correct order, you might corrupt that state and things start to fail. This is a classic example of the Hollywood principle; "don't call us, we'll call you".

Then they shouldn't be declared virtual and this documentation https://github.com/NancyFx/Nancy/wiki/The-Application-Before%2C-After-and-OnError-pipelines#overriding-methods-in-the-bootstrapper shouldn't suggest this approach is acceptable. From what you are saying, it offers an extension point which could modify the behaviour - Isn't that a violation of the open closed principal?

where are you passing your container to the bootstrapper? I can't see the container variable being used...

Apologies, I didn't add it to the example because I wanted to cut down on the length of my post. I'm using this in my bootstrapper....

protected override IContainer GetApplicationContainer()
{
    return ContainerWrapper.Instance;
}

Where ContainerWrapper allows me to share a container across applications as suggested here https://github.com/NancyFx/Nancy.Bootstrappers.StructureMap.

Anyway... I think I found the answer to my question in the docs... IRequestStartup https://github.com/NancyFx/Nancy/wiki/The-Application-Before%2C-After-and-OnError-pipelines#implementing-interfaces something like this....

public class MyRequestStart : IRequestStartup
{
    private readonly ICustomAuth _customAuthentication;

    public MyRequestStart(ICustomAuth customAuthentication)
    {
        if (customAuthentication == null)
        {
            throw new ArgumentNullException(nameof(customAuthentication));
        }

        _customAuthentication = customAuthentication;
    }

    public void Initialize(IPipelines pipelines, NancyContext context)
    {
        _customAuthentication.Authenticate(context);
    }
}

Cheers

[khellang]

If I change my bootstrapper to this...

That's a pretty contrived example. There are tons and tons of ways you could ruin your tests, either way...

My test still passes but my application wouldn't work... making my test pretty much pointless.

If you're worried by this, you need to assert that the context is the correct instance or that it satisfies a set of requirements. I mentioned a couple of examples above.

the route specified might easily have an effect on the test. If someone else changes the module, my test could fail for no good reason.

Again, this is, IMO, a moot point, and I think there is a good reason why your test fails. Either you're asserting on too much, or not the correct thing.

1. You don't have to write this test against an actual module. It could be a dummy module written specifically for this test.
2. There's no way to ever guarantee that if someone changes something somewhere, tests won't fail. That's a reason you have tests in the first place, right? If someone changes behavior, they should fail :smile:

Then they shouldn't be declared virtual

They're virtual because those are hooks that a bootstrapper implementation can override so the framework can call into it. The point is that these methods are called by the bootstrapper in the correct order and it comes with guarantees that (pretty much) whatever you do in there won't corrupt its state. This is what's called a Template Method Pattern.

[alexsaare]

That's a pretty contrived example. There are tons and tons of ways you could ruin your tests, either way...

The point of the example was to highlight what the test was trying to verify - the implementation. I'd say creating mock INancyContextFactory is also very contrived and way beyond the scope of what the test is trying to focus on.

Again, this is, IMO, a moot point, and I think there is a good reason why your test fails. Either you're asserting on too much, or not the correct thing.

I was trying to verify the implementation of RequestStartup in isolation so that I know my component behaves correctly according to specification defined - whatever that may be.

You don't have to write this test against an actual module. It could be a dummy module written specifically for this test.

This is a workaround to a problem which shouldn't exist in the context of the test I'm trying to create. No module should have an impact on a test which is testing the bootstrapper implementation - dummy or not. As indicated above IRequestStartup is what I was looking for and the test for the IRequestStartup implementation I declared is much more concise and would never be affected by any module.

[Test]
public void Initialize_Calls_NancyAuthentication_Authenticate_WithContext()
{
    // set up 
    var mockAuth = new Mock<INancyAuthentication>();
    var requestStartup = new NancyAuthenticationRequestStartup(mockAuth.Object);
    var mockPipeline = new Mock<IPipelines>();
    var context = new NancyContext();

    // exercise
    requestStartup.Initialize(mockPipeline.Object, context);

    // verify
    mockAuth.Verify(x => x.Authenticate(context), Times.Once);
}

There's no way to ever guarantee that if someone changes something somewhere, tests won't fail.

This is precisely why we should test the smallest unit of code possible. I believe the test you are suggesting and the tests I have shown are both equally required because they achieve very different things. The tests you are suggesting ensure that all the components used work together to meet the specification. The tests I have shown verify that the implementation of a single component meets it's specification.

They're virtual because those are hooks that a bootstrapper implementation can override so the framework can call into it. The point is that these methods are called by the bootstrapper in the correct order and it comes with guarantees that (pretty much) whatever you do in there won't corrupt its state. This is what's called a Template Method Pattern.

Cool, thanks for the more useful links. The code helps clarify your previous description which I misunderstood.

[damianh]

This is precisely why we should test the smallest unit of code possible.

http://dhickey.ie/2014/03/gullivers-travels-test/

Write outside tests and just enough inside ones.

[alexsaare]

Write outside tests and just enough inside ones.

"Just enough" is entirely subjective.

Whether this approach is a good thing or a bad thing depends on the potential impact of any defects in your application. My context is a trading platform responsible for over $1bn of daily activity which is regularly audited by 5 regulators. Whilst this is only a very thin UI in the overall architecture, the unintended consequences of small changes can be disastrous. Our use of TDD, BDD and code analysis (with what i'm sure you would consider to be suffocatingly high benchmarks) allows us to create a CI process that meets many of our regulatory requirements without employing an army of testers.

We may not deliver functionality with the same speed as you but what we deliver is bullet proof and the safety net our approach provides is invaluable.

[damianh]

I worked on trading platforms too, similar dollar value (Fixed Income mostly). Now its control of work safety systems in petrochemical. My opinion that outside (acceptance, BDD style) provide the most value, and, more often then not, adequate coverage, remains. On 6 Dec 2015 3:21 pm, "alexsaare" notifications@github.com wrote:

Write outside tests and just enough inside ones.

"Just enough" is entirely subjective.

Whether this approach is a good thing or a bad thing depends on the potential impact of any defects in your application. My context is a trading platform responsible for over $1bn of daily activity which is regularly audited by 5 regulators. Whilst this is only a very thin UI in the overall architecture, the unintended consequences of small changes can be disastrous. Our use of TDD, BDD and code analysis (with what i'm sure you would consider to be suffocatingly high benchmarks) allows us to create a CI process that meets many of our regulatory requirements without employing an army of testers.

We may not deliver functionality with the same speed as you but what we deliver is bullet proof and the safety net our approach provides is invaluable.

— Reply to this email directly or view it on GitHub https://github.com/NancyFx/Nancy/issues/2126#issuecomment-162320691.

[damianh]

With respect to Nancy, that is achieved via Nancy.Testing , or alternatively, OwinHttpMessageHandler.

[alexsaare]

Whist I agree that the BDD tests are of more value, this does not make the unit tests obsolete.

With respect to Nancy and my original post, I was looking for advice on unit testing code within request start up. As indicated above the IRequestStartup interface is what I was looking for.

You can close this issue now if you'd like.

I do appreciate your comments. Thanks for taking the time.

[damianh]

Very good. Btw we have a chat room on Jabbr for more direct convos. On 6 Dec 2015 9:02 pm, "alexsaare" notifications@github.com wrote:

Whist I agree that the BDD tests are of more value, this does not make the unit tests obsolete.

With respect to Nancy and my original post, I was looking for advice on unit testing code within request start up. As indicated above the IRequestStartup interface is what I was looking for.

I do appreciate your comments. Thanks for taking the time.

— Reply to this email directly or view it on GitHub https://github.com/NancyFx/Nancy/issues/2126#issuecomment-162342400.

[khellang]

Did we reach a consensus here?

[alexsaare]

Yes. IRequestStartup provides what I was looking for.

Thanks again.