Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

beforeStep is not called when using "meta steps" #454

Closed
jmauerhan opened this Issue · 5 comments

4 participants

@jmauerhan

When using the technique of having a step return an array of other steps, the function with the @beforeStep hook does not get executed before each of the "meta-steps" in the array.

Example:

/**
     * @Given /^I am logged in as an admin$/
     */
    public function iAmLoggedInAsAnAdmin()
    {
        return [
            new Step\Given('I visit "/admin/login-forced/admin"'),
            new Step\Given("I should see \"admin/dashboard\" in the URL"),
        ];
    }
    /** @BeforeStep */
    public function beforeStep(Event\StepEvent $event)
    {
        var_dump($event->getStep()->getText()); //For testing purposes
        $this->getSession()->wait(1000);

        //Wait for any ajax or animations to finish, with a max time.
        if ($this->getSession()->evaluateScript("return (typeof jQuery != 'undefined')"))
        {
            $this->getSession()->wait(60000, '(0 === jQuery.active && 0 === jQuery(\':animated\').length)');
        }
    }

Excerpt from my feature file:

Background:
    Given I am logged in as an admin

What I expect to see is a var_dump of all three of the steps:
Given I am logged in as an admin
I visit "/admin/login-forced/admin"
I should see "admin/dashboard" in the URL

What I actually see is only the print of "Given I am logged in as an admin"

@patwcummins

:+1: Good catch! This seems like an oversight

@stof
Owner

Well, the issue is that we could have different expectations here: currently, using a metastep is equivalent to calling the method directly on the context (well, a bit slower because it requires parsing the step and searching the method, but avoids bothering on the context defining it on the other hand).
The chained steps are purely an implementation detail.

Triggering the events for each substep introduces lots of issues.
And it cannot be introduced in 2.5 anyway, because it is a huge BC break

@jmauerhan

Hi stof,

Could you be a little more specific in what issues it introduces? I have been playing around and edited the code in the StepTester class to trigger the @BeforeStep and @AfterStep hooks, and I am not seeing any issues with my test suite - in fact it also gave us the added benefit of seeing the chained step in the "pretty" output, something else we wanted to try to do. (I am working on an extension to allow the rest of my team to use the change and avoid editing the vendor files directly). But after using it on our entire test suite, I did not get any different results than before, the same tests passed/failed, etc, the only change was our beforeStep function ran between each sub-step as well. Maybe it depends on what you have in your beforeStep?

Thanks so much for the quick response, I really appreciate it. I totally understand not adding it to the current version. Unfortunately we aren't planning to upgrade to 3.0 any time soon but do you know if this is still the case in 3.0? It sounds like it would be.

Edit: When I did make the change the output came out a little different than I expected, but it was easy to modify, and put the steps in the order I wanted.

@stof
Owner

@jmauerhan It will be an issue for any testsuite relying on the current behavior, i.e. getting the hook triggered only for real steps.
And the hook and the event must stay consistent (otherwise it will become impossible), and triggering the event for substeps will have a much bigger impact (breaking all formatters by displaying the chained steps in the output for instance).

Regarding 3.0, it does not have step chaining at all in core anymore. The goal is to implement it as an extension, but it does not work currently (the existing code is precisely facing the issue I'm describing about triggering these events too often). A refactoring is in progress to allow implementing the ChainedStepsExtension properly (expected to be submitted for review this week)

@everzet
Owner

@stof I'm almost done with that testers refactoring you asked. Today's evening - tomorrow morning will be a PR.

@everzet everzet closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.