A number of tests now use a custom getCookie method to get the current set cookies in a test to reuse them in a request using the Guzzle client.

I would be nice to have a easier way of doing this. Either add the method to BrowserTestBase to generate the cookie jar or maybe add an option to getHttpClient to set it there (not sure if this is possible, but would allow us to make this part of the API a little easier to find then a separate method I think).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lendude created an issue. See original summary.

ApacheEx’s picture

Status: Active » Needs review
FileSize
5.25 KB

I have moved this method to BTB (and removed duplications).
It seems add automatically cookies not so easy with current arch.
For sure we need a followup to apply it everywhere.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Ha, I just asked that in some other review! This sounds like an excellent idea.

Tess Bakker’s picture

Status: Reviewed & tested by the community » Needs work

Maybe we should call it getSessionCookies() or get ALL the cookies?

ApacheEx’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
6.69 KB
3.15 KB

It makes sense for me, thanks!
We will avoid conflicts if we want to have method to get all cookies.

Make it RTBC again, because it's just renaming and nothing more.

Lendude’s picture

@ApacheEx tsk, never RTBC your own patch, naughty naughty :)

RTBC++

ApacheEx’s picture

I got it. Thank you for notice this.

Tess Bakker’s picture

Status: Reviewed & tested by the community » Needs work

1. Nice patch
2. Almost perfect ;)

\Drupal\Tests\history\Functional\HistoryTest::getHttpClient can be removed / replaced.

Couldn't find any other implementation that looked (almost) identical.

ApacheEx’s picture

Status: Needs work » Needs review
FileSize
10.03 KB
3.2 KB

Yeah, I like it. Because no needed followup anymore.
All places where sessions cookies are used are replaced now.

p.s Thank you for your feedback.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Nice cleanup!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ff39393 and pushed to 8.6.x. Thanks!

diff --git a/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php b/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
index 694bf54deb..6f96971bb7 100644
--- a/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
+++ b/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
@@ -6,7 +6,6 @@
 use Drupal\Core\EventSubscriber\MainContentViewSubscriber;
 use Drupal\filter\Entity\FilterFormat;
 use Drupal\Tests\BrowserTestBase;
-use GuzzleHttp\Cookie\CookieJar;
 
 /**
  * Tests Quick Edit module integration endpoints.

REmoved unused use on commit.

  • alexpott committed ff39393 on 8.6.x
    Issue #2983504 by ApacheEx, Lendude, Tessa Bakker: Add a way to easily...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.