search

SpecFlowOSS / SpecFlow

#1 .NET BDD Framework. SpecFlow automates your testing & works with your existing code. Find Bugs before they happen. Behavior Driven Development helps developers, testers, and business representatives to get a better understanding of their collaboration
https://www.specflow.org/
Other
2.24k stars 754 forks source link

None of the DI container plugins satisfies the rules of context injection #2353

Open zadigus opened 3 years ago

zadigus commented 3 years ago

SpecFlow Version:

Used Test Runner

Version number: 3.11

Project Format of the SpecFlow project

.feature.cs files are generated using

Visual Studio Version

Enable SpecFlowSingleFileGenerator Custom Tool option in Visual Studio extension settings

Are the latest Visual Studio updates installed?

.NET Implementation:

Test Execution Method:

<SpecFlow> Section in app.config or content of specflow.json

None

Issue Description

I am currently developing a DI container plugin for ninject. In order to do so, I tried to get inspiration from the plugins that have already been developed for other DI containers, especially those maintained in this repository. I naively assumed that those plugins would entirely stick to all the rules enacted by SpecFlow, in particular those related to context injection.

The one rule that causes me a problem is the following:

As far as I can tell, this is not working for autofac. From what I can gather from its implementation, I see no reason why it would work for the Windsor DI container (I have not tested it personally). The reason why it works out-of-the-box with BoDi is because the IObjectContainer maintains a pool of instantiated objects and disposes those objects from that pool which are IDisposable. That kind of pool is for sure not standardly implemented in all DI containers, making it difficult to satisfy the above rule.

In the case of my plugin for ninject, which is 100% inspired by the autofac plugin implementation, the IKernel.Dispose() is not even triggered after a scenario has been executed. To make it happen, I've had to write a hook in my plugin:

    [Binding]
    public sealed class NinjectKernelDisposer
    {
        private readonly IKernel kernel;

        public NinjectKernelDisposer(IKernel kernel)
        {
            this.kernel = kernel;
        }

        [AfterScenario(Order=999999)]
        public void CleanupDisposables()
        {
            this.kernel.Dispose();
        }
    }

Then any specflow project using my ninject plugin should define the following specflow.json configuration:

{
  "stepAssemblies": [
    {
      "assembly": "MyCompany.Ninject.SpecFlowPlugin"
    }
  ]
}

With that in place, my DI container's Dispose() method gets fired automatically. Of course the above code works for scenario containers only. The same should be done for feature and thread containers too.

I am issuing this bug report for two reasons:

  1. for me it is a bug that plugins maintained in the SpecFlow repository do not satisfy the rules enacted by SpecFlow
  2. I would be happy to get some insights into possible solutions

What I am currently trying to do in my particular case is I am wrapping my DI container in such a way that I have a pool of instantiated disposable objects at hand. My wrapper changes the way my DI container's Resolve method is working: in addition to actually resolving object instances, it additionally maintains a pool of disposable instances. My container's Dispose method is also wrapped in such a way that it additionally disposes the objects contained in its pool of disposable objects.

Steps to Reproduce

  1. use autofac plugin (or any other DI container plugin)
  2. define a class inheriting IDisposable
  3. bind the disposable class from step 2 in your DI container
  4. inject the disposable class into a specflow binding (e.g. a step definition class or a hook)

Then, after the scenario, the disposable class does not get its Dispose() method called.

Repro Project

None

zadigus commented 3 years ago

Of course, a very easy way to solve this issue would be to relax the rule in the documentation. Why do you guys want that specflow be responsible for disposing the disposable objects? why shouldn't it be the tester's responsibility to take care of that?

SabotageAndi commented 3 years ago

I need to think about this, but I didn't yet found any time for it.

zadigus commented 3 years ago

Assuming you don't want not relax the rule in the documentation, because it would probably mean to change BoDi's behavior, a mecanism to call the container's Dispose() would be required. Currently, the only solution I've found to this problem is to

  1. add the following hook to my plugin's assembly:

    [Binding]
public sealed class NinjectKernelDisposer
{
private readonly IKernel kernel;

public NinjectKernelDisposer(IKernel kernel)
{
    this.kernel = kernel;
}

[AfterScenario(Order = 999999)]
public void CleanupDisposables()
{
    this.kernel.Dispose();
}
}
  2. add a reference to my plugin's assembly to my specflow project's app.config or specflow.json file:
{
  "stepAssemblies": [
    {
      "assembly": "MyCompany.Ninject.SpecFlowPlugin"
    }
  ]
}

However, that solution does not satisfy me because it means that the user of my plugin has to think about adding the reference to my plugin assembly to her app.config or specflow.json config file. It is very easy to forget. So a possibility would be that my plugin be packed in a nuget package which would then add this configuration or modify an existing app.config / specflow.json file. But again, that sounds more like a hack. Also, you surely have noticed the AfterScenario(Order = 999999). If the plugin's user defined an AfterScenario hook with higher order, then the Dispose would not be called at the right time.

For those reasons, I am wondering if it would not be possible for SpecFlow to expose new events in the RuntimePluginEvents that would be triggered whenever a scenario (for the scenario container), a feature (for the feature container), and a test thread (for the test thread container) is finished? or is there a better approach? e.g. with

runtimePluginEvents.ConfigurationDefaults += (sender, args) =>
{
  var pluginAssemblyName = Assembly.GetExecutingAssembly().GetName();
  args.SpecFlowConfiguration.AdditionalStepAssemblies.Add(pluginAssemblyName.Name);
};
zadigus commented 3 years ago

I have some code that I believe could be generalized for any DI container. I need to generalize it first and then I will propose it in a PR.

zadigus commented 3 years ago

@SabotageAndi I don't know if this should be the topic of another issue in this repository, but I noticed the following additional problem with the plugins that are published in this repo: they do not behave the same as if you used BoDi when it comes to register transient objects. It is also something that I was absolutely not aware of when I started using BoDi with SpecFlow. For example, until very recently, (but maybe I'm too naive), I thought that

this.objectContainer.RegisterTypeAs<Transient, ITransient>();

would register my object in the transient scope. I was not aware that BoDi with version <= 1.4 assumes that all objects are in a singleton scope. Therefore, a scenario like this:

Scenario: Injected transient dependencies are different in all SpecFlow bindings

        A transient dependency is an object that is created anew each time it is 
        being resolved. Changes in such objects made in a given SpecFlow binding
        should not be seen in another SpecFlow binding.

        Given I have injected Transient in the binding class StepClass1
        And the property MyProp of Transient has value 'value1' in StepClass1
        And I have injected Transient in the binding class StepClass2
        When I set property MyProp of Transient to 'value2' in StepClass2
        Then the property MyProp of Transient has value 'value1' in StepClass1

cannot succeed with BoDi 1.4, right? There is no way to define transient objects in BoDi 1.4, or am I seeing that wrong? Now, the autofac plugin has default scope transient, as documented here, but windsor has default scope singleton, as documented here, and I see no code in these plugins taking that difference into account (maybe I've overseen it). I would've expected autofac to put all the objects in the singleton scope, and I'm not sure, maybe this code is doing just that.

The reason why I am addressing this question is that I wrote my Ninject plugin for SpecFlow in such a way that it complies as much as possible with BoDi's behavior. In so doing, I noticed all the disposable dependencies I would register in BoDi would be disposed. I've just learned this week that they are disposed when I use BoDi just because they are all singletons. It seems like BoDi only disposes singletons automatically. Anything else is not disposed automatically. Therefore I am wondering if a DI plugin should dispose its registered disposable objects, when they aren't singletons. But if the plugin doesn't dispose the registered disposable objects, then it doesn't behave like BoDi <= 1.4 and therefore deviates from your documentation.

I upgraded my SpecFlow project to use BoDi 1.5.0 where I can define transient objects in the objectContainer. Now, in my opinion, your documentation lies about SpecFlow's behavior. Indeed, if I define the following hook:

    [Binding]
    public sealed class DependenciesConfigurator
    {
        private readonly IObjectContainer objectContainer;

        public DependenciesConfigurator(IObjectContainer objectContainer)
        {
            this.objectContainer = objectContainer;
        }

        [BeforeScenario]
        public void SetupScenarioContainer()
        {
            this.objectContainer.RegisterTypeAs<DisposableClass, IDisposable>().InstancePerDependency();
        }
    }
}

with the following DisposableClass

    public sealed class DisposableClass : IDisposable
    {
        private readonly FeatureContext featureContext;

        public DisposableClass(FeatureContext featureContext)
        {
            this.featureContext = featureContext;
        }

        public bool Disposed { get; private set; }

        public void Dispose()
        {
            this.Disposed = true;
            if (this.featureContext.HasTag("check-disposed-after-scenario"))
            {
                this.featureContext.Save(true, FeatureContextKeys.DisposableInstanceIsDisposed);
            }
        }
    }

then the SpecFlow scenario I've here is red, because DisposableClass.Dispose() is never called, i.e. it is wrong to state that 

If the injected objects implement IDisposable, they will be disposed after the scenario is executed.

However I turn the problem in my head, I only find inconsistent behavior and it would help me if you could make a statement on what the behavior should be. I find it pretty funny that a framework testing behavior be that unclear on its own behavior. But well, don't we say that "the shoemaker's children always go barefoot"?

There are two questions I'd need answers to:

  1. why do we want to inject only singletons into the binding classes? is that only to guarantee automatic disposal? if so, I think there is another solution and making all objects singletons is a bit "unexpected" or dangerous
  2. do we want to dispose all disposable objects that are used in specflow scenarios, even if they aren't singletons in the productive code?
zadigus commented 3 years ago

@SabotageAndi my tested version of a generalized plugin can be found here:

https://github.com/softozor/SpecFlowDiPlugin-mirror

I am currently still working on it, but I am close to the end. I you are interested in a PR, please let me know.