Specflow: [Discussion] - Improve ability to call steps from other steps.

[samholder]

I have a suggestion for improving how steps are called from other steps in Specflow and would like to solicit opinions on making this change before I jump in and do it, hopefully to try and make sure I don't fall into any previously discovered pits.

The main issue with calling steps from other steps is that you have to bind with text and this is awkward to refactor and to navigate in the code. Usually it means something like:

Given("I call my other step");

In the code. Specflows DI engine already supports passing in step classes into other step classes, but does not handle doing this through the constructor when there are circular dependencies (understandably). So currently this is possible:

```
[Binding]
public class SomeSteps
{
private readonly SomeOtherSteps someOtherSteps;

public SomeSteps(SomeOtherSteps someOtherSteps)
{
    this.someOtherSteps = someOtherSteps;
}

[Given("I do something")]
public void GivenIDoSomethign()
{
    someOtherSteps.DoSomethingFirst();
    //some other stuff
}

}

[Binding]
public class SomeOtherSteps
{
[Given("I do something first")]
public void DoSomethingFirst()
{
//some stuff
}
}

but I could not include a reference to `SomeSteps` in `SomeOtherSteps`

The proposal is to support the following scenario:

 ```
[Binding]
public class SomeSteps
{
    public SomeOtherSteps SomeOtherSteps {get;set;}

    public SomeSteps()
    {
    }

    [Given("I do something")]
    public void GivenIDoSomething()
    {
        someOtherSteps.DoSomethingFirst();
        //some other stuff
    }

    [Given("I do something else")]
    public void DoSomethingElse()
    {
        //some other stuff
    }
}

[Binding]
public class SomeOtherSteps
{

    public SomeSteps SomeSteps {get;set;}

    [Given("I do something first")]
    public void DoSomethingFirst()
    {        
        //some stuff
        SomeSteps.DoSomethingElse()
    }
}

By, waiting until after construction of the object graph and then setting the properties which have classes which are tagged [Binding] to be the instance the DI container contains.

This should allow us to remove calls to Given("I do something else"); and just call the methods directly, making building composite steps a lot simpler.

[BasHamer]

we solved this by making all steps use the same constructor & by keeping the state in the container;

    [Binding]
    public class InfrastructureSteps:StepBase
    {

        public InfrastructureSteps(IObjectContainer objectContainer) : base(objectContainer)
        {
        }
    }

And all state is kept in objects in the container.

in the base class there are lines like this
protected Pages Pages => ScenarioContext.Get<Pages>((typeof(Pages).FullName));
that provides access to the Pages object, so each step file uses the same Pages object. So none of our steps classes have any properties that are not coming from the ScenarioContext.

this is the stepbase setup

public abstract class StepBase
    {
        public StepBase(IObjectContainer objectContainer)
        {
            ObjectContainer = objectContainer;

        }

        protected IObjectContainer ObjectContainer { get; }
        protected ScenarioContext ScenarioContext { get => ObjectContainer.Resolve<ScenarioContext>(); }
        protected FeatureContext FeatureContext { get => ObjectContainer.Resolve<FeatureContext>(); }


        protected Interpeter Interpeter => ScenarioContext.Get<Interpeter>((typeof(Interpeter).FullName));
        protected ActionExecutor Executor => ScenarioContext.Get<ActionExecutor>((typeof(ActionExecutor).FullName));
        protected ILog Log => ScenarioContext.Get<ILog>((typeof(ILog).FullName));
        protected ObjectFactory ObjectFactory => ScenarioContext.Get<ObjectFactory>(typeof(ObjectFactory).FullName);
        protected TemplateManager TemplateManager => ScenarioContext.Get<TemplateManager>(typeof(TemplateManager).FullName);
        protected FileManager FileManager => ScenarioContext.Get<FileManager>(typeof(FileManager).FullName);

        protected void Register<T>(T item)
        {
            ScenarioContext.Add(typeof(T).FullName, item);
        }
    }

For true dynamic step execution I guess you'd need to get the TechTalk.SpecFlow.ITestRunner accessible from the steps themselves, I'm not aware that it is added to the IObjectcontainer.

[Tragedian]

Setter injection always leaves me with a bad taste in the mouth. It's unclear when you want a property to be set and when not setting it is a failure of some kind.

Rather than using setters, you could solve this problem in the same way as the ASP.NET Core framework: an accessor interface which allows late resolution of values:

public interface IStepAccessor<out T>
{
    T Steps {get;}
}

It just functions as a strongly-typed lookup from the container:

class StepAccessor<T> : IStepAccessor<T>
{
    private readonly IObjectContainer _container;

    StepAccessor(IObjectContainer container)
    {
        _container = container;
    }

    public T Steps => _container.Resolve<T>();
}

Usage is then really simple:

[Binding]
public class SomeOtherSteps
{

   public SomeSteps SomeSteps => _someStepsAccesor.Steps;

   public SomeOtherSteps(IStepsAccessor<SomeSteps> someStepsAccessor)
   {
       _someStepsAccessor = someStepsAccessor;
   }

   [Given("I do something first")]
   public void DoSomethingFirst()
   {        
       //some stuff
       SomeSteps.DoSomethingElse()
   }
}

[BasHamer]

I like it, that allows for state to be kept in the steps as well, something my approach does not allow for (this can be either a benefit or a drawback).

[SabotageAndi]

Personally I am also not a fan of property injection.

One technical problem I see here, is that I don't think we have a point, where we know, every step class is already instantiated. I am also not sure if BoDi has support for this.
@gasparnagy do you know this?

From organising your step code, I am not sure, if we want to go this way. If it is about calling other steps, we explained in the last years, that you shouldn't do this, but to use a driver pattern, to make it easier to combine your automation logic.

[gasparnagy]

In BoDi, there is a workaround: the IContainerDependentObject interface (this is implemented by the Steps base class BTW. If your class (or base class) implements this interface, BoDi notifies you through the IContainerDependentObject.SetObjectContainer method, so you can initialize any properties.
So you can make a base class that implements this and in the SetObjectContainer goes throuh the properties and sets them.

Generally calling step definitions from each other is pretty dangerous, because you can make up a very complex dependency chain and a small change in a step definiton will cause unwanted changes. It is a clearer solution to have a separate layer that encloses the automation logic (sometime called drivers, adapters or helpers), and do the reuse on that level.

[samholder]

Thanks for the input. I see little appetite for this and solutions above are sufficient for me

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.