Convert web tests to browser tests for history module

I have been working on this issue as you can see in the attached patch. Please have a look at my progress so far. It's not yet finished by for some reason I keep getting a 403 when executing getNodeReadTimestamps().

Anything I'm missing here?

// Perform HTTP request.
    $url = Url::fromRoute('history.get_last_node_view')
      ->setAbsolute();
    $this->verbose($this->serializer->encode($post, 'json'));
    return \Drupal::httpClient()->post($url->toString(), [
      'body' => $this->serializer->encode($post, 'json'),
      'headers' => [
        'Content-Type' => 'application/x-www-form-urlencoded',
      ]
    ]);

Also: do we have other examples?

Patch in #3 contained changes that weren't related to this issue.

You created a protected $client but weren't using it in the code. So I changed that. No idea why the 403 is happening.

The 403 is happening because the Guzzle request is from an unauthenticated (anon) user. After 5+ hours investigating this I'm not much closer to finding the solution, but at least we now know the problem!

I suspect the key is to pass cookies along with the body and headers (see patch), but I can't find a method to create a valid CookieJar. There's no point running this past the testbot - I can guarantee it will fail.

I think there is also an issue with the format of the node_ids being posted, because the call fails even when the user restrictions are removed from getNodeReadTimestamps().

Updated title and summary to refer to the parent issue, #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase).

1. Converted from Web Test to Browser Test.
2. Replace cUrl (not in Web Test Base) with Guzzle.

+++ b/core/modules/history/tests/src/Functional/HistoryTest.php
@@ -41,6 +53,14 @@ protected function setUp() {
+    $domain = parse_url($this->getUrl(), PHP_URL_HOST);
+    $session_id = $this->getSession()->getCookie($this->getSessionName());
+    $opts = [
+      'cookies' => CookieJar::fromArray([$this->getSessionName() => $session_id], $domain),
+      'http_errors' => FALSE,
+    ];
+    $this->client = \Drupal::service('http_client_factory')->fromOptions($opts);

I think this setup is not necessary. We already have a Guzzle client in the Mink driver we are using for browser tests. $this->getSession()->getDriver()->getClient()->getClient(); should give you the Guzzle client, right?

We really need an ApiTestBase for such cases, see #2812967: API test base: Provide a test helper to test REST and similar APIs.

Not sure what is better for this issue, instantiating a new Guzzle client or reusing the existing one from Mink.

How about this then? It now uses the existing Guzzle client from Mink rather than instantiating a new one.

The cookies are the vital thing that means the Guzzle calls are successful. The could (probably) be obtained for each Guzzle request individually, but it is more efficient to do so in the set-up.

FYI some of the functionality is based on that in modules/system/tests/src/Functional/CsrfRequestHeaderTest.php.

The patch in #14 don't apply to https://www.drupal.org/project/drupal/releases/8.4.x-dev I downloaded today (7th march). I am uploading the updated patch.

I am doing my first reroll. In #14 i did it manually. In this I use git.

1. Created getHttpClient() method.
2. Yes, the cookies are definitely required - that is the crux of the issue. The http request has no innate knowledge of the session created in setUp() so we have to provide it. If you remove the cookies from the post request it will not work.

Also added the @see mentioned in #13.

Yes, I used $this->client = \Drupal::service('http_client_factory')->fromOptions($opts); in #10, but @klausi advised using $this->getSession()->getDriver()->getClient()->getClient(); instead in #11.

The cookies are ugly, I tried some variations myself with setting it on the Guzzle client but it also didn't work for me. We might find something better in the future, but for now this is already a great improvement and even makes the test code much more readable.

1. +++ b/core/modules/history/tests/src/Functional/HistoryTest.php
   @@ -33,6 +38,20 @@ class HistoryTest extends WebTestBase {
   +  /**
   +   * The Guzzle HTTP client.
   +   *
   +   * @var \GuzzleHttp\Cookie\CookieJar;
   +   */
   +  protected $cookies;
   

   doc block is wrong, this is not he client. Should be "The cookie jar holding the testing session cookies for Guzzle requests."

2. +++ b/core/modules/history/tests/src/Functional/HistoryTest.php
   @@ -121,28 +132,42 @@ public function testHistory() {
   +   * @return \GuzzleHttp\Client
   +   */
   

   Description missing, should be "The client with BrowserTestBase configuration."

1. Corrected the doc block.
2. Added the description.

Looks good!

Uploading the same patch created with "git diff -M5%" to prove that the file has moved.

Committed and pushed fce944d to 8.4.x and af000d8 to 8.3.x. Thanks!

Follow-up to convert remaining test #2898437: Convert HistoryTimestampTest to kernel test