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.

Sign up for GitHub

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

Big pipe compatibility follow-up #325

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+56 −2
Open

Big pipe compatibility follow-up #325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9814b6
Test messages
zaporylie Nov 15, 2016
9bdaa01
Add BigPipeContext
zaporylie Nov 15, 2016
84b1ae4
Merge branch 'master' into follow/259
zaporylie Jul 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions behat.yml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ drupal8:
- Drupal\DrupalExtension\Context\MinkContext
- Drupal\DrupalExtension\Context\MarkupContext
- Drupal\DrupalExtension\Context\MessageContext
- Drupal\DrupalExtension\Context\BigPipeContext
filters:
tags: "@d8&&~@d8wip"
extensions:
Expand All @@ -129,3 +130,4 @@ drupal8:
content: "#content"
selectors:
error_message_selector: '.messages--error'
success_message_selector: '.messages--status'
20 changes: 18 additions & 2 deletions features/messages.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,33 @@ Feature: Ensure that messages are working properly on local installs
In order to be sure that Drupal 8 with Big Pipe enabled can be tested
Messages are tested against a local installation

Scenario: Non-JS messages
Scenario: Non-JS messages for anonymous
Given I am on "/user/login"
When I fill in "a fake user" for "Username"
And I fill in "a fake password" for "Password"
When I press "Log in"
Then I should see the error message "Unrecognized username or password"

@javascript
Scenario: JS messages
Scenario: JS messages for anonymous
Given I am on "/user/login"
When I fill in "a fake user" for "Username"
And I fill in "a fake password" for "Password"
When I press "Log in"
Then I should see the error message "Unrecognized username or password"

Scenario: Non-JS messages for authenticated
Given I am logged in as a user with the "access site in maintenance mode,administer site configuration" permissions
And I am on "/admin/config/development/maintenance"
When I check the box "maintenance_mode"
And I press "Save configuration"
Then I should see the message "The configuration options have been saved."

@javascript
Scenario: JS messages for authenticated
Given I am logged in as a user with the "access site in maintenance mode,administer site configuration" permissions
And I am on "/admin/config/development/maintenance"
When I check the box "maintenance_mode"
And I press "Save configuration"
And print last response
Then I should see the message "The configuration options have been saved."
36 changes: 36 additions & 0 deletions src/Drupal/DrupalExtension/Context/BigPipeContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

namespace Drupal\DrupalExtension\Context;

use Drupal\big_pipe\Render\Placeholder\BigPipeStrategy;
use Behat\Mink\Exception\UnsupportedDriverActionException;

/**
* Big Pipe context.
*/
class BigPipeContext extends RawDrupalContext {

/**
* Prepares Big Pipe NOJS cookie if needed.
*
* @BeforeScenario
*/
public function prepareBigPipeNoJsCookie()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

}
catch (UnsupportedDriverActionException $e) {
// Set NOJS cookie.
$this
->getSession()
->setCookie(BigPipeStrategy::NOJS_COOKIE, true);

}
catch (\Exception $e) {
// Mute exceptions.
}
}

}