Upgrade to Behat 3 / Drupal Extension 3

The basics are working, but the tests are running faster than CTools can render its modals. This is supposed to be handled by the AfterStep function. But for some reason, it doesn't run as a tagged hook. It only runs if you remove the @javascript tag.

If you remove the tag, the function throws an exception because it is expecting a Behat\Behat\Hook\Scope\AfterStepScope parameter, but we're sending a StepEvent parameter. It looks like we can't use this function in the same way.

Looks like this is going to be the replacement for the afterStep function:

  /**
   * @AfterStep
   */
  public function afterStep($scope) {
    if ($scope->getTestResult()->getResultCode() === 0) {
      $this->iWaitForAJAX();
    }
  }

assuming we can get the tagged scenarios to work and add the @javascript tag.

It appears that BeforeStep and AfterStep hooks no longer work with tags? See https://github.com/Behat/docs/pull/55. So putting the @javascript tag in the @AfterStep annotation will cause it to be skipped.

If the untagged AfterStep hook, as currently coded, tries to run on a non-javascript scenario, it will throw an error.

This could be fixed if there were a way to check whether the parent scenario is tagged and act accordingly.

OK, this is working for Javascript scenarios and not affecting non-Javascript scenarios:

  /**
   * @BeforeScenario @javascript
   *
   * Set a variable so AfterStep knows this is a javascript scenario.
   */
  public function setJavascript() {
    $this->javascript = true;
  }

  /**
   * @AfterScenario @javascript
   *
   * Unset the variable so other scenarios don't run the javascript AfterStep.
   */
  public function unsetJavascript() {
      $this->javascript = false;
    }

  /**
   * @AfterStep
   */
  public function afterStep($scope) {
    if ($scope->getTestResult()->getResultCode() === 0 && isset($this->javascript)) {
      $this->iWaitForAJAX();
    }
  }

I'll be working on this today, aiming for a patch in the afternoon.

Proposed architecture:

PanopolyContext includes all (and only) helper methods. No step definitions. That way it can safely be extended by all of the .inc files.

The inc files extend PanopolyContext. Each contains a set of related step definitions.

FeatureContext is empty and extends PanopolyContext.

Open questions:
What does PanopolyContext extend? RawDrupalContext or RawMinkContext? Either way, some helper methods may need to be added.

Should inc files containing steps related to a specific feature such as WYSIWYG be moved to that feature?

Patch attached. There's a remaining issue on some of the "wait for live preview" steps.

Updated patch attached. Everything is passing locally.

There is one hitch remaining: Right now we are pointing our subcontext paths for discovery at profiles/panopoly/modules/. Because of where the panopoly_test module is located, this will include the vendor directory. As of the latest Behat Drupal extension, this will cause a fatal PHP error because an outdated documentation snippet is being discovered and parsed. See https://github.com/jhedstrom/drupalextension/issues/97.

We can't assume that this will be fixed in a way that won't still cause issues for us. So we should make sure to exclude the vendor directory from the subcontext include path. One way to do that would be to put the panopoly_test module into a different subdir from the rest of the Panopoly modules. A patch to drupal-org.make that does that is also attached. This would also require changes to travis-ci.sh - included in the patch.

The issue https://github.com/jhedstrom/drupalextension/issues/97 has been fixed by renaming the doc snippet. The patch to the distribution makefile is no longer needed - hiding it.

Here's a new patch that updates to the Behat Drupal Extension 3.0.10. Notes on some changes:

• I had to change the files_path in a way that made more sense to put it in the behat.template instead of behat.common, so it can be changed in your local instance.
• Subcontexts can no longer extend PanopolyContext without causing errors about functions defined twice. That makes me think we should throw everything into one or the other of our subcontexts.
• Passing in escaped characters is different, I'm not sure how to do it anymore. This required changes to the hidden view mode options feature and the image alt encoding test in the wysiwyg_media feature.
• For some reason, live preview wasn't always triggering on the table widget and spotlight widget, so I hacked in a step to change the title field to force the preview to update.
• The content item widget test suffers from some timing issues in the "content as content" scenario. It takes a while for the Save button to wake up after the test selects Teaser mode. Unless/until we figure this out, we need a Wait step in there.

Tests are passing locally. We'll see what Travis has to say: https://travis-ci.org/cboyden/panopoly/builds/63911476

Travis of course needs some more changes. Environment variables now need to be JSON, and the behat.travis.yml file needs updating.

Needed to adjust the way the BEHAT_PARAMS variable was set in the Travis script. New profile patch attached. Build is here: https://travis-ci.org/cboyden/panopoly/builds/63999010.

About item 1: No, not really. The custom step "Given I am viewing a landing page" used the createNode function from DrupalContext, which is not available when extending RawDrupalContext. And we can't extend DrupalContext because we'll get every step in it throwing an "already defined" exception. So rather that rewriting that step definition in our code, I chose to use the built-in step. I'll see if there's an alternative that will allow us to use a one-liner in the scenario instead of a table.

Well, we have the Drupal API! I wouldn't mind using it to programmatically create a new node, even if that's duplicating code that's already in the Behat Drupal extensions somewhere.

I disagree, I think we create ourselves more work, especially since: a) now a landing page is just another node type; and b) when we start testing Drupal 8 we'll have to recreate all of our custom steps for that. I'd rather use the built-in steps when they're available, and even push some of our custom steps (managed file comes to mind) back into the Behat Drupal Extension, or into Panels and other modules, so they can be of broader use.

Or, it could be possible to implement communication between contexts as outlined here: http://docs.behat.org/en/latest/cookbooks/context_communication.html

That way the custom Given should be able to call createNode.

Part of the problem with the Live Preview test appears to be that config fields for different widget types have different IDs, and labels. Some examples are:

• The Table widget title field ID is "edit-title" and it has label of "Title."
• The Content List widget title field ID is "edit-widget-title" and there is no label (the text "Title" looks like a label, but it's just a span).

This is probably due to differences between Views widgets and FPP widgets.

Also there could be timing issues at work here. Maybe the "wait for javascript" function isn't sufficient to detect the changes to the fields, and the test is getting ahead of itself.

Here's a new patch that addresses some issues from comment #16 (2,4,5,6, and 7), and also attempts to fix the Live Preview test failures.

Here also is a new patch using the approach to BEHAT_PARAMS suggested by @dsnopek. Travis build with this and the test patch above: https://travis-ci.org/cboyden/panopoly/builds/64035090

Some of the live preview scenarios are failing consistently every time on Travis - but not all of them. It could be one of several reasons:

1. The text isn't being successfully entered into the form fields, even though the step reports as passed
2. Live preview is not being triggered
3. The step "wait for live preview" is reporting as passed before live preview has actually finished

Or maybe there are other possibilities.

This is all passing locally. Without being able to see what's happening on the screen at each step, I'm not sure how to get this working.

Is there a way to force live preview to update?

The test passed, but it's reporting a step as undefined: "And I select the first autocomplete option...", which is in the content item widget scenario.

80 scenarios (79 passed, 1 undefined)
1146 steps (1138 passed, 3 undefined, 5 skipped)
21m12.30s (61.90Mb)
--- FeatureContext has missing steps. Define them with these snippets:
    /**
     * @When I select the first autocomplete option for :arg1 on the :arg2 field
     */
    public function iSelectTheFirstAutocompleteOptionForOnTheField($arg1, $arg2)
    {
        throw new PendingException();
    }

Runtime is creeping up again, particularly in the Firefox scenarios. Is there something we can do to the "wait for Ajax" or "wait for live preview" functions to speed them up?

When running the tests locally, I'm getting some failures:

--- Failed scenarios:

    features/advanced_widgets.feature:7
    features/basic_ipe.feature:12
    features/contentitem_widget.feature:53
    features/contentitem_widget.feature:87
    features/panopoly_magic/livepreview.feature:7
    features/panopoly_magic/livepreview.feature:132

80 scenarios (74 passed, 6 failed)
1145 steps (1080 passed, 6 failed, 59 skipped)

advanced_widgets only failed the once. basic_ipe fails every time (the "Phelan" link isn't available when the test tries to click it). contentitem_widget fails occasionally, sometimes in different places. livepreview also fails regularly, sometimes in different places.

These are all timing issues. Some places that are prone to failure are when saving the page after having saved a pane update, when waiting for the "customize" button to appear on a new landing page, or when waiting for live preview to finish.

1. +++ b/tests/behat.template.yml
   @@ -3,10 +3,15 @@ imports:
   +      subcontexts:
   +        paths:
   +          - "./profiles/panopoly"
   

   This is in behat.common - does it need to be here? Should it be removed from behat.common?

2. +++ b/tests/behat.travis.yml
   @@ -3,12 +3,18 @@ imports:
   +      subcontexts:
   +        paths:
   +          - "./profiles/panopoly"
   

   As above in behat.template.

3. +++ b/tests/features/contentpage.feature
   @@ -23,6 +23,7 @@ Feature: Add content page
   +      And I wait 5 seconds
   

   Maybe this could be shortened. It's one of the common timing failure points, so I don't think it can be removed entirely.

4. +++ b/tests/features/contentpage.feature
   @@ -34,6 +35,7 @@ Feature: Add content page
   +      And I wait 5 seconds
   

   As above for the too-small scenario.

5. +++ b/tests/features/hidden_view_mode_options.feature
   @@ -31,7 +31,7 @@ Feature: Hidden view mode options
   +      And I fill in "Hidden view mode options" with "diff_standard"
   

   This was because I couldn't figure out how to send in escaped characters like \n.

6. +++ b/tests/steps/panopoly_test.behat.inc
   @@ -474,86 +410,215 @@ class TestSubContext extends BehatContext implements DrupalSubContextInterface {
   +   * @Then I should see :text in the :tag element in the :region region
   

   Ideally this would go into the Drupal extension, it's one of a piece with all of the other steps in MarkupContext. But it's fine here for now.

7. +++ b/tests/steps/panopoly_test.behat.inc
   @@ -474,86 +410,215 @@ class TestSubContext extends BehatContext implements DrupalSubContextInterface {
   +   * @Then I should see an image in the :region region$/
   

   The trailing $/ is left over from a regex and can be removed.

Hmm, weird! All those tests pass for me locally. What versions of Firefox, Selenium and PHP are you running? Since Travis had timing issues with an older Selenium, that's my best guess at the moment.

I'm on Selenium 2.44. There were some issues between Firefox and Selenium a while back, so I had to keep an older version of Firefox around just for testing. It sounds like those must have been resolved. I'll try updating both.

Edit: Basic IPE test is no longer failing, but I'm still getting occasional failures on live preview and content item widget. This is with Selenium 2.45, Firefox 38, PHP 5.4.38 on the command line and 5.4.8 on the server.

This is looking good! I'm still getting timing-related failures on the content item widget and live preview features, but since they're running OK on Travis, we could probably chalk that up to local gremlins and move forward with this. If they weren't occasionally failing in different places, I might say we should add a "wait" step.

And you tried updating your Firefox and Selenium to the latest stable versions?

Yes, I'm on Selenium 2.45 and Firefox 38.0.1.

I've run the problematic tests on Chrome a few times, and it seems to go much better. No failures yet on contentitem or livepreview.