From 61dfababfb11b296e5e2e26e35407d7de8778f2a Mon Sep 17 00:00:00 2001 From: Samuel Demirdjian Date: Wed, 22 Jul 2015 11:18:26 +0300 Subject: [PATCH 1/3] Restructure Request tests to avoid Request::factory calls in providers `Request::factory()` calls in test "provider" methods initializes the `Request::$initial` static variable, affecting other test methods, even if those "provider" methods are not direct providers of those other test methods. In our case the `HTTPTest::test_redirect` is affected because the `HTTP::redirect` method needs the `Request::$initial` static variable to be available. Even if PHPUnit is run with the `--filter=HTTPTest` flag, the "provider" methods of `RequestTest` are still available, `Request::factory()` calls are still made, and the `Request::$initial` static variable is still initialized. This becomes problematic because `HTTPTest::test_redirect` is now dependant of the `RequestTest` "providers", as those "providers" change the global state in favor of its needs. Once `RequestTest` "providers" are not available -- ex: by deleting RequestTest.php file from disk -- `HTTPTest::test_redirect` test cases fail. In order to not alter the global state, and to not affect other tests, static variables initialization, and calls to methods that affect static variables should not be made in test case providers. Rather calls should be done carefully in `setUp` and `tearDown` methods, as well as in the tests methods themselves. In this case I have removed `Request::factory()` calls from providers and restructed the tests. --- tests/kohana/RequestTest.php | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/kohana/RequestTest.php b/tests/kohana/RequestTest.php index bb5f74b7a..390e33885 100644 --- a/tests/kohana/RequestTest.php +++ b/tests/kohana/RequestTest.php @@ -523,18 +523,18 @@ public function provider_headers_get() { $x_powered_by = 'Kohana Unit Test'; $content_type = 'application/x-www-form-urlencoded'; + $request = new Request('foo/bar', array(), TRUE, array()); return array( array( - $request = Request::factory('foo/bar') - ->headers(array( + $request->headers(array( 'x-powered-by' => $x_powered_by, 'content-type' => $content_type ) ), - array( - 'x-powered-by' => $x_powered_by, - 'content-type' => $content_type + array( + 'x-powered-by' => $x_powered_by, + 'content-type' => $content_type ) ) ); @@ -566,7 +566,6 @@ public function provider_headers_set() { return array( array( - Request::factory(), array( 'content-type' => 'application/x-www-form-urlencoded', 'x-test-header' => 'foo' @@ -574,7 +573,6 @@ public function provider_headers_set() "Content-Type: application/x-www-form-urlencoded\r\nX-Test-Header: foo\r\n\r\n" ), array( - Request::factory(), array( 'content-type' => 'application/json', 'x-powered-by' => 'kohana' @@ -589,13 +587,13 @@ public function provider_headers_set() * * @dataProvider provider_headers_set * - * @param Request request object * @param array header(s) to set to the request object * @param string expected http header * @return void */ - public function test_headers_set(Request $request, $headers, $expected) + public function test_headers_set($headers, $expected) { + $request = new Request(TRUE, array(), TRUE, array()); $request->headers($headers); $this->assertSame($expected, (string) $request->headers()); } From 70b88e8749576d181ebd44cb5481a09009dfeee8 Mon Sep 17 00:00:00 2001 From: Samuel Demirdjian Date: Wed, 22 Jul 2015 11:58:26 +0300 Subject: [PATCH 2/3] When initial request is not available parse $_GET --- classes/Kohana/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/Kohana/Request.php b/classes/Kohana/Request.php index 2fe0f29e8..72e0affee 100644 --- a/classes/Kohana/Request.php +++ b/classes/Kohana/Request.php @@ -668,7 +668,7 @@ public function __construct($uri, $client_params = array(), $allow_external = TR $uri = array_shift($split_uri); // Initial request has global $_GET already applied - if (Request::$initial !== NULL) + if (Request::$initial === NULL) { if ($split_uri) { From c37cf6f261f5a3793878c3d76489bb664bd42cd8 Mon Sep 17 00:00:00 2001 From: Samuel Demirdjian Date: Wed, 22 Jul 2015 12:50:11 +0300 Subject: [PATCH 3/3] Properly setUp and tearDown HTTPTest Initialize `Request::$initial` for HTTPTest::test_redirect. After initializing `Request::$initial`, it is probably better to set it to `NULL` instead of using `$this->_initial_request` to temporary save the value. But I prefered to keep things in conformity with `RequestTest` for the time being. --- tests/kohana/HTTPTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/kohana/HTTPTest.php b/tests/kohana/HTTPTest.php index 94c123deb..33ce72672 100644 --- a/tests/kohana/HTTPTest.php +++ b/tests/kohana/HTTPTest.php @@ -15,6 +15,8 @@ */ class Kohana_HTTPTest extends Unittest_TestCase { + protected $_inital_request; + /** * Sets up the environment */ @@ -24,8 +26,21 @@ public function setUp() { parent::setUp(); Kohana::$config->load('url')->set('trusted_hosts', array('www\.example\.com')); + $this->_initial_request = Request::$initial; + Request::$initial = new Request('/'); } + /** + * Tears down whatever is setUp + */ + // @codingStandardsIgnoreStart + public function tearDown() + // @codingStandardsIgnoreEnd + { + Request::$initial = $this->_initial_request; + parent::tearDown(); + } + // @codingStandardsIgnoreStart /** * Defaults for this test