Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BeforeStep and AfterStep no longer support tags in Behat 3 #347

Merged
merged 1 commit into from
Jan 30, 2017

Conversation

pfrenssen
Copy link
Collaborator

Fixes #92.

@pfrenssen pfrenssen merged commit f3c1f46 into master Jan 30, 2017
@pfrenssen
Copy link
Collaborator Author

Also cherry-picked to the 3.2 branch: c174852

@pfrenssen pfrenssen deleted the no-tags-for-hooks branch January 30, 2017 11:57
@jonathanjfshaw
Copy link
Contributor

I'm not sure this is a good enough solution. The fix covers the case if a feature is tagged@javascript, but not if a scenario is tagged @javascript. And that difference is against usual Behat DX (in that we expect scenarios to inherit tag effects from their parent features) and is bound to catch people out.

According to @everzet to get the expected result we have to "Use hooks and temporary state in your context (or trait). That's nasty, but will work.".

So how about:

  private $taggedJavascript;

    /**
    * @BeforeScenario @javascript
    */
   public function beforeScenario(BeforeScenarioScope $scope)
   {
      $this->taggedJavascript = true;
   }

   /**
    * @AfterScenario @javascript
    */
   public function afterScenario(AfterScenarioScope $scope)
   {
       $this->taggedJavascript = false;
   }

  /**
   * @BeforeStep
   */
  public function beforeJavascriptStep($event) {
    if ($this->taggedJavascript) {
      $text = $event->getStep()->getText();
      if (preg_match('/(follow|press|click|submit)/i', $text)) {
        $this->iWaitForAjaxToFinish();
      }
  }

@pfrenssen
Copy link
Collaborator Author

I didn't know this, sounds like a good suggestion. Unfortunately this PR is already merged, would it be possible to create a new one for this?

@jonathanjfshaw
Copy link
Contributor

Sure. This gives us 4 javascript specific functions. Do you want to split these out into a javascript context for 4.x?

@pfrenssen
Copy link
Collaborator Author

Yes a separate context for this sounds like a really good idea.

@jonathanjfshaw
Copy link
Contributor

We don't currently have a feature that tests this. Somewhere in the Drupal admin UI that uses AJAX could provide a test.

How about:

@javascript
Given I am logged in as an "administrator"
When I visit "/admin/structure/types/manage/page/form-display"
Then I should see not "Widget settings: Textfield"
When I press the "title_settings_edit" button
Then I should see "Widget settings: Textfield"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants