Big pipe compatibility follow-up #325

zaporylie · 2016-11-15T20:33:35Z

#259 doesn't test messages for authenticated users. This follow-up PR should prove that messages work only for anonymous users.

In next commits to branch I will suggest how can we fix it (based on workaround proposed here: #258 (comment))

zaporylie · 2016-11-15T21:04:44Z


The command "vendor/bin/behat -fprogress --strict" exited with 1.
103.19s$ vendor/bin/behat -fprogress --profile=drupal${DRUPAL_VERSION} --strict
...................................................................... 70
...................................................................... 140
...................................................................... 210
.............F.....F

--- Failed steps:
    Then I should see the message "The configuration options have been saved." # features/messages.feature:27
      The page 'http://127.0.0.1:8888/admin/config/development/maintenance?q=admin/config/development/maintenance' does not contain any messages (Exception)
    Then I should see the message "The configuration options have been saved." # features/messages.feature:36
      The page 'http://127.0.0.1:8888/admin/config/development/maintenance?q=admin/config/development/maintenance' does not contain any messages (Exception)

44 scenarios (42 passed, 2 failed)
230 steps (228 passed, 2 failed)
1m42.51s (94.91Mb)
The command "vendor/bin/behat -fprogress --profile=drupal${DRUPAL_VERSION} --strict" exited with 1.
0.00s$ test ${DRUPAL_VERSION} -ne 7 || vendor/bin/behat -fprogress --profile=drush --strict

Extra test scenarios proved DrupalExtension doesn't work well for Authenticated user and BigPipe enabled.

In next commit I will suggest how can we fix it.

zaporylie · 2016-11-15T21:27:48Z

The command "vendor/bin/behat -fprogress --strict" exited with 1.
92.04s$ vendor/bin/behat -fprogress --profile=drupal${DRUPAL_VERSION} --strict
...................................................................... 70
...................................................................... 140
...................................................................... 210
..................F

--- Failed steps:
    Then I should see the message "The configuration options have been saved." # features/messages.feature:35
      The page 'http://127.0.0.1:8888/admin/config/development/maintenance?q=admin/config/development/maintenance' does not contain any messages (Exception)

44 scenarios (43 passed, 1 failed)
229 steps (228 passed, 1 failed)
1m31.47s (95.28Mb)

As you can see first failing step (Non-JS messages for authenticated) has been fixed.

In the same time extra scenarios have revealed that [email protected] doesn't work well with BigPipe. I tested that scenario locally with PhantomJS and it works perfect. Did you consider switching from zombiejs to phantomjs?

mglaman · 2017-03-20T01:48:40Z

I copied the BigPipeContext into my project and it solved my woes. My project also uses PhantomJS vs zombiejs

jhedstrom · 2017-12-05T18:07:28Z

Sorry for the long delay in getting to this. It looks like a great start! It needs to be rebased/merged with the latest master, and from the looks of it, there were some JS failures too. Thanks for working on this!

Berdir · 2017-12-05T23:18:13Z

Is there no official way to check if a driver supports JS or not?

Using a try/catch for that is not exactly a clean way. Also wondering if this really should be an optional context, everyone will have to figure out then that this exists and add it to this project, should at least be documented then somehere.

I'll also ping Wim Leers about this.

jhedstrom · 2017-12-06T17:34:35Z

Is there no official way to check if a driver supports JS or not?

From what I can tell, no. The JS drivers don't implement any additional interfaces or anything to differentiate themselves from the non-JS drivers...

Berdir · 2017-12-06T17:38:03Z

That's pretty unfortunate then, in my project I could obviously just check the actual driver class since I know what it is.

Could we maybe somehow store this on a property or so so we don't have to throw and catch an exception for every single scenario? I guess it won't make a measurable difference

The thing is that big pipe should be compatible with non-JS clients by redirecting them through a meta tag and then sets that cookie itself IIRC. But it looks like the driver doesn't support that.

wimleers · 2017-12-06T18:02:05Z

Is there no official way to check if a driver supports JS or not?

That'd be great!

But I think catching UnsupportedDriverActionException and then setting the no-JS cookie is a pretty clever hack.

The thing is that big pipe should be compatible with non-JS clients by redirecting them through a meta tag and then sets that cookie itself IIRC. But it looks like the driver doesn't support that.

Correct. But if the driver doesn't support that, then it's violating one of the foundational features of web browsers: it doesn't respect <noscript><meta http-equiv="Refresh" content="0; URL=http://example.com/somewhere"></noscript>. Which means that this driver doesn't support graceful degradation for non-JS clients. Which sounds like a huge oversight.

Finally: when does Behat run the assertions? Does it wait for the load event to complete? I'm asking because otherwise tests using BigPipe could run into random failures depending on how fast the server responded, and depending on how fast the JS was executed.

jhedstrom · 2017-12-12T19:49:00Z

I've merged the latest master for this over in #445.

jhedstrom · 2017-12-12T19:53:43Z

I opened FriendsOfPHP/Goutte#329 for the Goutte browser.

jhedstrom · 2017-12-12T21:30:08Z

In local testing, it would appear that the attempt to just set the cookie in a @BeforeScenario hook won't work, since when a user logs in, they get a new session that loses this cookie for some reason.

This PR was squashed before being merged into the 4.2-dev branch (closes #27118). Discussion ---------- [BrowserKit] Adds support for meta refresh | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27117  | License | MIT | Doc PR | symfony/symfony-docs#...  This adds support for following redirects defined by the `http-equiv=refresh` meta tag, such as ``` <meta http-equiv="Refresh" content="0; URL=http://example.com/somewhere"> ``` Additional background at jhedstrom/drupalextension#325 (comment)  Commits ------- 1c64c82 [BrowserKit] Adds support for meta refresh

davereid · 2018-07-13T15:57:22Z

We are definitely experiencing this a lot recently:

Finally: when does Behat run the assertions? Does it wait for the load event to complete? I'm asking because otherwise tests using BigPipe could run into random failures depending on how fast the server responded, and depending on how fast the JS was executed.

leymannx · 2018-12-17T21:16:53Z

After applying the patch it solved a pretty mean issue for me. #258 (comment)

jhedstrom · 2018-12-17T23:10:48Z

Previously this didn't seem to resolve the issue (see my comments above regarding losing cookies on login), but if it's working for others, and we can get the tests passing, I'm all for this workaround until the fix in browserkit above makes its way into Mink, etc.

leymannx · 2018-12-27T22:16:24Z

Worth mentioning the patch results in an exception to be thrown on @api tagged scenarios when the BigPipe module is not enabled.

  ┌─ @BeforeScenario # Drupal\DrupalExtension\Context\BigPipeContext::prepareBigPipeNoJsCookie()
  │
  ╳  Fatal error: Class 'Drupal\big_pipe\Render\Placeholder\BigPipeStrategy' not found (Behat\Testwork\Call\Exception\FatalThrowableError)

I'm actually quite surprised by that behavior. How smart this extension is that it won't load the class of a disabled module?!

After re-enabling BigPipe it works just fine again.

Berdir · 2020-01-29T23:21:25Z

src/Drupal/DrupalExtension/Context/BigPipeContext.php

+     *
+     * @BeforeScenario
+     */
+    public function prepareBigPipeNoJsCookie()


I did have something like this in a custom project, but it stopped working with behat-extension 4, due to the authentication manager. That now does a fast logout unconditionally before login which kills the session and with that, the cookie.

I actually already did the same in my overriden logout() method with drupal-extension 3 but I thn also re-set the cookie there.

I switched to an @afterstep and adding the cookie if I have a user and the cookie isn't there yet, but that doesn't seem like a very efficient approach.

Maybe we can solve it by emitting an event when we do the fast logout in the authentication manager?

vever001 · 2020-10-23T17:17:12Z

Thanks for this example @Berdir
On my side, using v4.0.1 and v4.1.0 it didn't work with a BeforeScenario.
I have overridden the login method and set the cookie there, which seems to work for now.

bsuttis · 2021-03-15T21:59:55Z

@vever001 can you post how you overrode the login method?

vever001 · 2021-03-15T22:44:51Z

Sure, this is the context I added to the behat.ymlin default.suites.default.contexts section:

<?php

declare(strict_types = 1);

namespace Drupal\Tests\my_namespace\Behat;

use Behat\Mink\Exception\UnsupportedDriverActionException;
use Drupal\big_pipe\Render\Placeholder\BigPipeStrategy;
use Drupal\DrupalExtension\Context\DrupalContext as DrupalExtensionDrupalContext;

/**
 * Provides step definitions for interacting with Drupal.
 */
class DrupalContext extends DrupalExtensionDrupalContext {

  /**
   * {@inheritdoc}
   */
  public function login(\stdClass $user) {
    parent::login($user);
    $this->disableBigPipe();
  }

  /**
   * Disables Big Pipe.
   *
   * Workaround for issue with drupalextension and big_pipe.
   * See https://github.com/jhedstrom/drupalextension/issues/258
   */
  protected function disableBigPipe() {
    try {
      // Check if JavaScript can be executed by Driver.
      $this->getSession()->getDriver()->executeScript('true');
    }
    catch (UnsupportedDriverActionException $e) {
      // Set NOJS cookie.
      $this
        ->getSession()
        ->setCookie(BigPipeStrategy::NOJS_COOKIE, TRUE);
    }
    catch (\Exception $e) {
      // Mute exceptions.
    }
  }

}

bsuttis · 2021-03-16T00:55:58Z

@vever001 thanks, it's working!

FlorentTorregrosa · 2021-03-19T17:20:29Z

Hello,

Edit:

drupal/drupal-extension: 4.1.0
drupal/core-recommended: 9.1.5
drupal/core-dev: 9.1.5

I am writting (my almost first) Behat tests on a comment form for anonymous users and I wanted to test that after submitting the form, the approbation message is displayed and the admin receives an email.

So I encountered this issue.

I have not tested the code of this PR yet.

#325 (comment) is not working for me because in my case, I am with an anonymous user. So I guess the @BeforeScenario of the conext in the PR will be better.

And I have another problem, if I write a context in a module, either in src or tests/src with proper namespace then referencing it in my behat.yml, the context is not detected. Is there something special to do?

I find out that for example Search API has a search_api.behat.inc file but I would really like to avoid .inc files. And no problem for a custom context inside a features/bootstrap folder from wher the behat.yml file is located.

I can't find anything about that in the documentation.

Thanks for any help.

FlorentTorregrosa · 2021-03-23T10:46:18Z

Hello,

I add manually tested the new context. It fixed my usage. Thanks!

Waiting for the PR to be merged when possible.

pfrenssen · 2022-03-08T19:59:46Z

src/Drupal/DrupalExtension/Context/BigPipeContext.php

+    {
+        try {
+            // Check if JavaScript can be executed by Driver.
+            $this->getSession()->getDriver()->executeScript('true');


We were using a similar approach (https://github.com/drupaltest/behat-traits/blob/8.x-1.x/src/Traits/BrowserCapabilityDetectionTrait.php#L42) but this doesn't work with ChromeDriver which throws a fatal error when no page has been loaded yet (which is the case when running @BeforeScenario)

Test messages

c9814b6

Add BigPipeContext

9bdaa01

Merge branch 'master' into follow/259

84b1ae4

jhedstrom changed the title ~~Followup to #259~~ Big pipe compatibility follow-up Dec 5, 2017

jhedstrom added the bug label Dec 5, 2017

jhedstrom added this to the 4.0 release milestone Dec 5, 2017

jhedstrom mentioned this pull request Dec 5, 2017

LoggedIn() doesn't wait for page to load #364

Closed

This was referenced Dec 6, 2017

Add an interface for drivers that support JavaScript minkphp/Mink#752

Open

Reroll: Big pipe compatibility follow-up #445

Closed

jhedstrom closed this Dec 12, 2017

jhedstrom mentioned this pull request Dec 12, 2017

Add support for noscript and meta tag refresh FriendsOfPHP/Goutte#329

Closed

jhedstrom reopened this Dec 12, 2017

This was referenced May 1, 2018

[BrowserKit] Add support for meta refresh redirects symfony/symfony#27117

Closed

[BrowserKit] Adds support for meta refresh symfony/symfony#27118

Merged

jhedstrom mentioned this pull request May 16, 2018

Follow meta refresh tags (when BrowserKit does) Behat/MinkExtension#326

Open

leymannx mentioned this pull request Dec 17, 2018

Compatibility with big_pipe #258

Open

jhedstrom modified the milestones: 4.0 release, 4.1 release Jul 23, 2019

Berdir reviewed Jan 29, 2020

View reviewed changes

pfrenssen reviewed Mar 8, 2022

View reviewed changes

bkosborne mentioned this pull request Feb 21, 2024

Content loaded via BigPipe is sometimes not available by the time steps are executed, leading to random failures #660

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Big pipe compatibility follow-up #325

Big pipe compatibility follow-up #325

zaporylie commented Nov 15, 2016

zaporylie commented Nov 15, 2016

zaporylie commented Nov 15, 2016

mglaman commented Mar 20, 2017

jhedstrom commented Dec 5, 2017

Berdir commented Dec 5, 2017

jhedstrom commented Dec 6, 2017 •

edited

Loading

Berdir commented Dec 6, 2017

wimleers commented Dec 6, 2017

jhedstrom commented Dec 12, 2017

jhedstrom commented Dec 12, 2017

jhedstrom commented Dec 12, 2017

davereid commented Jul 13, 2018

leymannx commented Dec 17, 2018

jhedstrom commented Dec 17, 2018

leymannx commented Dec 27, 2018

Berdir Jan 29, 2020

pfrenssen Mar 8, 2022

vever001 commented Oct 23, 2020

bsuttis commented Mar 15, 2021

vever001 commented Mar 15, 2021 •

edited

Loading

bsuttis commented Mar 16, 2021

FlorentTorregrosa commented Mar 19, 2021 •

edited

Loading

FlorentTorregrosa commented Mar 23, 2021

pfrenssen Mar 8, 2022

Big pipe compatibility follow-up #325

Are you sure you want to change the base?

Big pipe compatibility follow-up #325

Conversation

zaporylie commented Nov 15, 2016

zaporylie commented Nov 15, 2016

zaporylie commented Nov 15, 2016

mglaman commented Mar 20, 2017

jhedstrom commented Dec 5, 2017

Berdir commented Dec 5, 2017

jhedstrom commented Dec 6, 2017 • edited Loading

Berdir commented Dec 6, 2017

wimleers commented Dec 6, 2017

jhedstrom commented Dec 12, 2017

jhedstrom commented Dec 12, 2017

jhedstrom commented Dec 12, 2017

davereid commented Jul 13, 2018

leymannx commented Dec 17, 2018

jhedstrom commented Dec 17, 2018

leymannx commented Dec 27, 2018

Berdir Jan 29, 2020

Choose a reason for hiding this comment

pfrenssen Mar 8, 2022

Choose a reason for hiding this comment

vever001 commented Oct 23, 2020

bsuttis commented Mar 15, 2021

vever001 commented Mar 15, 2021 • edited Loading

bsuttis commented Mar 16, 2021

FlorentTorregrosa commented Mar 19, 2021 • edited Loading

FlorentTorregrosa commented Mar 23, 2021

pfrenssen Mar 8, 2022

Choose a reason for hiding this comment

jhedstrom commented Dec 6, 2017 •

edited

Loading

vever001 commented Mar 15, 2021 •

edited

Loading

FlorentTorregrosa commented Mar 19, 2021 •

edited

Loading