Must verify:

always
  1. the response headers required for BigPipe (\Drupal\big_pipe\EventSubscriber\HtmlResponseBigPipeSubscriber::onRespond()
  2. works with all types of placeholders (look at #2674334: BigPipeStrategy test coverage + remaining @todos for a comprehensive list, including representative edge cases): test on a page with all these types
when JS is on
  1. presence of big_pipe/big_pipe asset library
  2. AJAX loading of new asset libraries
  3. placeholder replacement order
when JS is off
  1. absence of big_pipe/big_pipe asset library
  2. in-line loading of new asset libraries using HTML
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Status: Active » Needs review
FileSize
20.09 KB

Here is a first iteration. This tests 2 out of the 5 types of placeholders I want to test. And it only tests the "JS on" case, not the "JS off" case yet.

But the bulk of the test infrastructure is here, I should be able to finish this swiftly tomorrow morning.

I'll continue working on this tomorrow morning.

Status: Needs review » Needs work

The last submitted patch, 2: 2675670-2.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
20.57 KB
4.73 KB

Fixed those failures. Both due to expectations assuming Drupal is installed in a subdirectory.

At the same time got rid of all that hardcoded JSON. Instead keeping more legible data structures in PHP that are converted to JSON when testing.

Wim Leers’s picture

FileSize
21.47 KB
5.95 KB

Addressed

// @todo verify positions of $placeholder_replacement_positions, $start_signal_position, $stop_signal_position

Also improved test debug output, which should help a lot in making these tests and their results easier to understand & maintain.

Wim Leers’s picture

FileSize
22.22 KB
3.23 KB

One less @todo:

      // 3. HTML attribute value subset placeholder: CSRF token in link.
-    // @todo

90% of the time to get this one working was spent on this beauty:

    // Ensure we can generate CSRF tokens for the current user's session.
    $session_data = $this->container->get('session_handler.write_safe')->read($this->cookies[$this->getSessionName()]['value']);
    $csrf_token_seed = unserialize(explode('_sf2_meta|', $session_data)[1])['s'];
    $this->container->get('session_manager.metadata_bag')->setCsrfTokenSeed($csrf_token_seed);

Oh, Symfony…

Wim Leers’s picture

FileSize
22.85 KB
2.41 KB

And another @todo gone:

     // 4. Edge case: custom string to be considered as a placeholder that
     // happens to not be valid HTML.
-    // @todo

This one was painless (and glorious, check out the markup it generates!)

Wim Leers’s picture

FileSize
26.76 KB
7.94 KB

And here's then the corresponding BigPipe no-JS test.

Wim Leers’s picture

FileSize
27.18 KB
1.79 KB
+++ b/src/Tests/BigPipeNoJsDetectionTest.php
@@ -221,6 +217,63 @@ class BigPipeNoJsDetectionTest extends WebTestBase {
+      '<div data-big-pipe-nojs-placeholder-id="callback=Drupal%5CCore%5CRender%5CElement%5CStatusMessages%3A%3ArenderMessages&amp;args[0]&amp;token=a8c34b5e"></div>' => '<marquee>Yarhar llamas forever!</marquee>',

I defined the wrong expectation here. That's not the status message markup.

The actual expected markup (patch attached) would also verify the additional CSS that gets loaded, which means when JS is off's second point in the IS is then also verified.

Wim Leers’s picture

FileSize
28.5 KB
4.12 KB

First round of clean-up:

  • rename BigPipeNoJsDetectionTest to BigPipeTest
  • fix & improve test docblocks
Wim Leers’s picture

FileSize
28.78 KB
1.33 KB

Update big_pipe_test.module so that \Drupal\big_pipe\Tests\BigPipeTest::testNoJsDetection() no longer needs to install the session_exists_cache_context_test module.

Wim Leers’s picture

FileSize
29.32 KB
2.54 KB

Improve docs/helper function name for the special <meta> refresh handling.

Wim Leers’s picture

FileSize
46.06 KB
33.53 KB

This was already using the same five test cases as the unit tests were. This reroll makes that more maintainable by centralizing all aspects of all five test cases in a single class, that then both the unit test and the integration tests can call out to.

Wim Leers’s picture

FileSize
46.21 KB
14.83 KB

Final clean-up. The test logic of \Drupal\big_pipe\Tests\BigPipeTest::testBigPipe() and \Drupal\big_pipe\Tests\BigPipeTest::testBigPipeNoJs() is now much easier to follow, and should be clear on at least a high level to all readers.

  • Wim Leers committed afef6b2 on 8.x-1.x
    Issue #2675670 by Wim Leers: BigPipe response delivery test coverage...
Wim Leers’s picture

Status: Needs review » Fixed

lgtm

Fabianx’s picture

RTBC in retrospective

Status: Fixed » Closed (fixed)

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