Comments

michielnugter created an issue. See original summary.

andypost’s picture

Status: Active » Needs review
FileSize
2.39 KB

Initial conversion

Status: Needs review » Needs work

The last submitted patch, 3: 2870457-3.patch, failed testing.

dawehner’s picture

For me it would be totally fine to inline these 3 lines of code needed for \Drupal\simpletest\WebTestBase::setHttpResponseDebugCacheabilityHeaders
It is used twice in the entirety of core.

michielnugter’s picture

Component: phpunit » page_cache.module
Issue tags: +phpunit initiative
andypost’s picture

Status: Needs work » Needs review
FileSize
861 bytes
2.92 KB

Inlined cacheability disabling

dawehner’s picture

Thank you @andypost
I strongly hope this passes it now :) Maybe still open up an issue to add such a method to something, it can't hurt.

andypost’s picture

@dawehner do you mean to extend AssertLegacyTrait or separate trait cos usage after patch is

core8$ git grep setHttpResponseDebugCacheabilityHeaders
core/modules/simpletest/src/WebTestBase.php:2184:  protected function setHttpResponseDebugCacheabilityHeaders($value = TRUE) {
core/modules/system/src/Tests/Routing/RouterTest.php:96:    $this->setHttpResponseDebugCacheabilityHeaders(FALSE);

Status: Needs review » Needs work

The last submitted patch, 7: 2870457-7.patch, failed testing.

naveenvalecha’s picture

Status: Needs work » Needs review
FileSize
4.72 KB
1.8 KB

Here's the patch which added the drupalHead function to the BTB base class to get it's passed. However, the tests are failing on local due to the headers response code in testConditionalRequests. The request is expecting not modified(304) but the drupalGet function in BTB is returning 200 which I'm not sure why it's happening

//Naveen

Status: Needs review » Needs work

The last submitted patch, 11: 2870457-10.patch, failed testing. View results

naveenvalecha’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.29 KB
4.03 KB

PageCacheTest module requires the couple of assertions which needs to be added before converting them.

BTB drupalGetHeader function is calling getResponseHeader of Mink Session class which internally changing the string to lowercase which making couple of tests fail.

    public function getResponseHeader($name)
    {
        $headers = $this->driver->getResponseHeaders();

        $name = strtolower($name);
        $headers = array_change_key_case($headers, CASE_LOWER);

        if (!isset($headers[$name])) {
            return null;
        }

        return is_array($headers[$name]) ? $headers[$name][0] : $headers[$name];
    }

How about overriding this method in BTB? Let's continue this discussion in follow-up issue and let keep this issue for minimal disruption.

//Naveen

Lendude’s picture

Status: Needs review » Needs work

@naveenvalecha hmmm not sure if it's worth splitting this off, since that just leaves 1 test in this patch. I would opt for trying to get PageCacheTest to pass here. If it really needs new methods to be added, lets make an issue for that, then we just postpone on that issue.

nlisgo’s picture

Status: Needs work » Needs review
FileSize
4.81 KB
718 bytes

Status: Needs review » Needs work

The last submitted patch, 15: convert_web_tests_to-2870457-15.patch, failed testing. View results

andypost’s picture

Status: Needs work » Needs review
FileSize
3.69 KB
7.25 KB

replace curl with guzzle + fix docblock

btw it makes sense to split this out of the test to separate unit

Status: Needs review » Needs work

The last submitted patch, 17: 2870457-page_cache_tests-17.patch, failed testing. View results

dawehner’s picture

Status: Needs work » Needs review
FileSize
9.88 KB
2.94 KB

This fixes one of the failures.

Status: Needs review » Needs work

The last submitted patch, 19: 2870457-19.patch, failed testing. View results

dawehner’s picture

Status: Needs work » Needs review
FileSize
10.67 KB
3.1 KB

I couldn't figure out testNoUrlNormalization yet.

Status: Needs review » Needs work

The last submitted patch, 21: 2870457-21.patch, failed testing. View results

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.