Specflow: RFC: Remove Calling steps from steps

[SabotageAndi]

Currently it is possible to call steps from steps.
Docs for this feature: https://specflow.org/documentation/Calling-Steps-from-Step-Definitions/

For that you have to inherit from https://github.com/techtalk/SpecFlow/blob/master/TechTalk.SpecFlow/Steps.cs to have access to the methods.

We will mark these methods as obsolete with SpecFlow 3.1 and we will remove them in SpecFlow 4.0.

Reason behind is:

• feature makes problems with implementing upcoming cucumber-messages
• feature will be removed also from other cucumber implementations
• feature assists in bad habbits of writing steps

We generally recommend the use of the driver pattern (Good blogpost: http://leitner.io/2015/11/14/driver-pattern-empowers-your-specflow-step-definitions/).
With that, you don't get to the idea to call other steps directly or via string.

[SabotageAndi]

FYI @gasparnagy

[BasHamer]

From a backward-compatibility perspective, you could add ITestRunner to the IOC container.

[SabotageAndi]

ITestRunner is the entry point of the SpecFlow runtime. The IOC Container is created by it. But it is registered in it. You can resolve it from it, but you can't register your own implementation.
But in this special case, I really want to break backward- compatibility. Calling in this way other steps is simply worst practice. No idea why we implemented it.

[gasparnagy]

I am fine with this. These methods are anyway not much in use...

[pebermyntelars]

Where does this leave us with combined steps in general?
Call the step function directly? In my local case, we've got a lot of tests that reuse same partial procedures in various end to end scenarios.

[jameswtelfer]

I guess this means that using the framework to create combined steps is going away entirely, and I either put the implementation code into an injected object and call it from the different steps, or call the step methods directly?

Calling directly doesn't work for me as the steps are organised into different classes to keep them a sensible size. The Driver Pattern referenced is really just a way of putting this logic into a new class and injecting that into the steps. That doesn't seem better to me, just a different way of keeping the code in the step methods themselves small.

Given the reasons this is going being driven by better integration with the rest of the cucumber ecosystem this seems like a sensible change. It's just going to cost me a bunch of work to rearchitect my tests.

[gasparnagy]

@jameswtelfer I would like to highlight, that we are only remove this option from the public interface. While we believe that the other alternatives we have mentioned (e.g. driver classes) are better in general than this approach was, the internal infrastructure will still support evaluating string-based steps, if needed (this is what the generated code-behind files use anyway). So to be able to make an existing implementation work with newer SpecFlow versions, you will be able to find a workaround. You can look at how, the Steps base class implements this methods currently: it delegates to the ITestRunner interface, that can be injected to any class. (You can even put this to your own base class if you wish.)

The goal behind this change was to drive away new projects from this approach and to simplify the complexity of the newly introduced reporting interface.

FYI: you can also let step definition objects injected to each-other. So even if you don't want to introduce a driver layer, you can still inject the dependent step definition classes to others and with that you can call the step definition methods directly.

[jameswtelfer]

FYI: you can also let step definition objects injected to each-other. So even if you don't want to introduce a driver layer, you can still inject the dependent step definition classes to others and with that you can call the step definition methods directly.

Stupidly I hadn't thought of this possibility, which fixes my short-term need to migrate and not break things, while long-term moving away from this pattern. Thanks for the nudge @gasparnagy.

[Moshex]

Question on "feature assists in bad habits of writing steps"
Does this concept mostly apply to web-based automation or all automation frameworks?
The reason I ask this is that within Mobile automation since every test has the same common steps to get to a certain point of the application to be able to test the specific feature you would use a "Given" step to be able to direct the Gherkin to be more focused on the feature you are testing and not the setup for the feature. If you take out this ability to me it seems like it would lead to worse code practices where the same steps keep getting rewritten instead of just having the step written once then calling the Gherkin every time you wish to use that code making less violate the DRY principle of development.
I guess I would like a little more concept as to what is the better practice here since I am not sure how driver pattern works or if that even sounds like a better solution then what is already in place.
Here is an example of what I am talking about:

  [Given("I am on change password page")]

        public void GivenIAmOnChangePasswordPage()
        {
            Given("I am on the Dashboard");
            When("I select More from the bottom navigation");
            Then("I should see more menu options");
            When("I choose Change Password");
        }`

How would you change this when the implementation of removing Gherkin in the steps file is removed? Keep in mind that all of these steps are just setup steps that don't need to be called at the feature-based level

Thank you for your help

[SabotageAndi]

@Moshex you can get the other classes with your bindings via Context Injection and call the methods direct.

[Moshex]

@SabotageAndi using the example that I have provided how would that now be written? or if you have a code snippet example of using Context Injection in this manner that would be of help as well.
Thank you,

[jameswtelfer]

@Moshex as I understand it, the suggested solution for your example would look something like the following.

First define a stand-alone class with methods which implement the actions for your steps (or common bits of them):

class StepImplementation {
   public void GoToDashboard() { ... }
   public void SelectMore() { ... }
   public void CheckMenuOptions() { ... }
   public void SelectChangePassword() { ... }
}

Then have a very thin class with all the steps which is given an instance of the implementation class at construction time:

[Binding]
class Steps
{
  private readonly StepImplementation _si;

  public Steps(StepImplementation si)
  {
    _si = si;
  }

  [Given("I am on change password page")]
  public void GivenIAmOnChangePasswordPage()
  {
    _si.GoToDashboard();
    _si.SelectMore();
    _si.CheckMenuOptions();
    _si.SelectChangePassword();
  }

  [Given("I am on the Dashboard")]
  public void GivenIAmOnTheDashboard()
  {
    _si.GoToDashboard();
  }

  ...
}

At least, this is my interpretation of the pattern described. It is a little extra work, but it does mean that the compiler catches errors when things get changed, rather than your tests failing at run-time because someone decided that Dashboard should be written dashboard for example.

[Moshex]

@jameswtelfer Thank you so much for your example from what you have stated I was able to get a working solution using my existing framework what I have is multiple steps class, and I called the step class that holds the method in the class that needs the method. Here is a quick code example with just the 1st method replaced:

    [Binding]
    public class ChangePasswordSteps 
    {
        readonly IApp app;
        readonly Pages.ChangePasswordPage _ChangePasswordPage;
        private readonly DashboardSteps _dashboardSteps;

        public ChangePasswordSteps(DashboardSteps dashboardSteps)
        {
            //Resolve App from Feature Container.  This is added with the [Setup] method.
            app = FeatureContext.Current.Get<IApp>("App");
            //Resolve pages that are set up with the AppInitializer InitializeScreens Method
            _ChangePasswordPage = FeatureContext.Current.Get<Pages.ChangePasswordPage>("ChangePasswordPage");
            _dashboardSteps = dashboardSteps;
        }

        [Given("I am on change password page")]
        public void GivenIAmOnChangePasswordPage()
        {
            _dashboardSteps.GivenIAmOnTheHomePage();
            //Given("I am on the Dashboard");
            When("I select More from the bottom navigation");
            Then("I should see more menu options");
            When("I choose Change Password");
        }

Now whether or not this is better I feel is up for a debate, I get the spelling thing but we already have to deal with that on the feature file-level and as Gherkin users, we know to be very careful with our strings :p
Now I am not sure if this is what was intended but if this direction is what the spec flow team is looking for
this is a big refactor consideration when moving up to spec flow version 3.2. I would still like to know more about how this concept is the better practice verse what is already implemented but for now, I am glad I have a solution.

[Santjo]

How would you change this without modify the related scenarios or a separte step for each using?

It is used as a general step to execute any passed "Then-Step" (which already exists) after confirmation of message.

        [Then(@"(.*) after confirmation of the last message")]
        public void ThenExecAnyThenStepUponMessageConfirmation(string anyThenStep)
        {
            // .... any code which confirms a message box
            // ....
            log.Debug("Clicked OK button on the message box");
            Then(anyThenStep);
        }

Thus was possible to pass any existing Then step without creating a new specific handling or step.
e.g.
...
Then _the device should be stopped_ after confirmation of the last message
Then _the result should be 42_ after confirmation of the last message
...

[noeltupas]

@SabotageAndi What about the ScenarioContext and FeatureContext properties in Steps.cs? We have several large test suites that use ScenarioContext.Current and refactoring to use ScenarioContext directly would ease the transition. Are there plans to deprecate those properties as well?

[daveh551]

I think there's something here that I'm not getting, and I hope I'm not out of order in asking this here.
First, I have a testing scenario (end-to-end regression testing) where I have several different pages that need to be tested, and the pages are invoked in different combinations from different operations on the home page. The scenario for any individual page may vary from several steps to 20 or 30 or more.
Using the DRY principle of software engineering, it seems very logical to me to write a test scenario for each of the subordinate pages, and then invoke those scenarios from the different operations on the home page.
Now, I THINK I can figure out how to do that using an injected TestRunner class as @gasparnagy mentions above, but I get the distinct impression from this whole discussion that this approach, however it's implemented, is frowned upon and discouraged.
I've looked at the paper on the Driver pattern (and have been unsuccessful in finding any other articles describing the use of that pattern), and I just don't get it. I don't understand the connection between watering flowers and testing multiple subordinate scenarios.
If someone could enlighten me or point me to more detailed explanations, I would be very grateful. Thanks.

[gasparnagy]

@daveh551 When you have two step definitions, A and B and B needs to call A, that means that A contains some automation logic that is needed by both A and B. The idea of the driver pattern is to move this shared automation logic into a new method (C) in a new class (Driver1), so both A and B can call C. In simple cases you could even make C a static method so you can simply call it from A and B (Driver1.C()). But you can make C an instance method as well (a method that requires an instance of Driver1). In this case you can let SpecFlow create the Driver1 instance for you by injecting it to the classes where A and B placed.

So basically we resolved a dependency B -> A by introducing C, where A -> C and B -> C. This new set of classes that contain the automation logic are used to call driver (other names: adapter, helper) and therefore it is called the driver pattern. It is nothing new, the page object pattern that many teams use is the same: page object is a specialized driver that can automate a page.

[Moshex]

@gasparnagy if it's okay I would like to point out a different perspective using your example. if you have methods "a" and "b" that share the same steps however method b has additions that are now applicable to the current scenario. without this functionality being removed you would call method "a" in method "b" with your needed added code. However, once the ability to call a step from another step is removed you are now required to have a method c in order to not violate the DRY principle. This makes to where more code and overhead is needed to do the same thing that has been in spec-flow for years. with regards to dependency on "a" that just switches to a dependency on "c", both of the methods listed have a dependency on "c" and if something is changed in "c" it affects both methods. However, if there are just an "a" and "b" methods and something is changed in "b" then "a" is left unaffected. This is just going with your example I can think of many other ways that being able to call a step in another step is helpful.
Thank you for your insight I hope to have this discussion to get a greater understanding for all of us :)

[gasparnagy]

@Moshex Just to make it clear: when we were talking about calling steps from each-other, we were talking about invoking a string-based API (pass in the text of the step). This is very badly traceable: it is nearly impossibly to find from which other step the step was called from. Calling a step from another _as method_ is still possible and not deprecated.

So this leaves you with three options:

1. Incrementally transform towards the driver pattern, that will led to a layering that has been proved to be useful and maintainable in many projects, but it requires you a bit of extra work. This is essentially separating the automation concern from the testing concern. You have to change the drivers if and only if the underlying code has to be automated differently and you need to change the step definitions when you would like to translate the same English sentence to a different testing actions. I have gone through this transition (introducing driver pattern) in multiple projects and so far this was the improvement that caused the most benefits in my automation solutions. Of course this does not mean that it is helpful for everyone. Hence you have option 2.

2. You can call step definitions from each other as methods. If the step definition methods are in the same class this is easy. If they are in different classes, you can make them static (works only if they don't manage state) or inject the other step definition classes into the one you would like to call it from. (Circular dependencies are bad anyway, but if you really need such, you can inject the IObjectContainer to the class and resolve the instance of the other step definition class on the fly,) And just like before: _Be very careful - calling steps from each other usually causes a messy code and unwanted side effects of changes!_. By calling these as methods, you can at least use your dev tooling (Visual Studio) to find usages and trace dependencies.

3. If you have a larger codebase that uses the text-based "call steps from each other" pattern and it would be hard or not cost efficient to change, you can use the workaround I have mentioned earlier by injecting the ITestRunner. All warnings of option 2 are also apply to this one as well of course.

[gasparnagy]

To answer the unanswered comment of @noeltupas: No, ScenariContext and FeatureContext is absolutely fine on Steps base class, there is no plan to remove them. Accessing scenario and feature related information from the step definitions is a valid concern, especially if it is about tracing (what is the current scenario, step that is currently running) or tag-related automation logic (what tags are attached to the current scenario). Using ScenariContext for sharing data (state) across step definitions might cause problems (it is recommended to use context injection instead), but this is not related to the Steps base class anyway.

[Moshex]

@gasparnagy I need a little more clarification on this statement "This is very badly traceable: it is nearly impossibly to find from which other step the step was called from" I just do a search using Visual Studio for the Gherkin string and it works every time, do you have more reasons as to why you feel that statement is true? Are there other IDE's that have issues with doing a search on the Gherkin string?

Also switching a method to "static" works really well I want to thank you. @gasparnagy this is the best suggestion on how to handle the removal of this feature. While there will be a lot of work to refactor my code base for this new pattern at least the essence of what I am trying to accomplish can still be preserved.

[gasparnagy]

@Moshex "very badly traceable": search by text is possible of course, but error prone, becomes difficult when the steps had parameters and requires the entire team to form the calls in a certain way (building up the step text from multiple strings, parameter substitution, etc.). In contrast for method calls, VS can even highlight if no one is calling or how many calls you have for that method. Maybe the words "nearly impossible" were a bit too strong, but forgive me for that -- I have spent far too much of my life tracing down such codes... :)