From f5f3d21875a9421318fff212dc0189d40540bf32 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:20:26 +0200 Subject: [PATCH 1/9] HeadersTest: rearrange the method order in the test class ... grouping tests related to array access together. --- tests/Response/HeadersTest.php | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index a4888436d..8930092a4 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -7,6 +7,15 @@ use WpOrg\Requests\Tests\TestCase; final class HeadersTest extends TestCase { + + public function testInvalidKey() { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Object is a dictionary, not a list'); + + $headers = new Headers(); + $headers[] = 'text/plain'; + } + public function testArrayAccess() { $headers = new Headers(); $headers['Content-Type'] = 'text/plain'; @@ -21,6 +30,14 @@ public function testCaseInsensitiveArrayAccess() { $this->assertSame('text/plain', $headers['content-type']); } + public function testMultipleHeaders() { + $headers = new Headers(); + $headers['Accept'] = 'text/html;q=1.0'; + $headers['Accept'] = '*/*;q=0.1'; + + $this->assertSame('text/html;q=1.0,*/*;q=0.1', $headers['Accept']); + } + /** * @depends testArrayAccess */ @@ -42,19 +59,4 @@ public function testIteration() { } } } - - public function testInvalidKey() { - $this->expectException(Exception::class); - $this->expectExceptionMessage('Object is a dictionary, not a list'); - $headers = new Headers(); - $headers[] = 'text/plain'; - } - - public function testMultipleHeaders() { - $headers = new Headers(); - $headers['Accept'] = 'text/html;q=1.0'; - $headers['Accept'] = '*/*;q=0.1'; - - $this->assertSame('text/html;q=1.0,*/*;q=0.1', $headers['Accept']); - } } From bc07207a7e10c19d95ae523e88f49ad030eb70ab Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:51:37 +0200 Subject: [PATCH 2/9] HeadersTest: rename a test to be more descriptive --- tests/Response/HeadersTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index 8930092a4..9a0a87e39 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -8,7 +8,7 @@ final class HeadersTest extends TestCase { - public function testInvalidKey() { + public function testOffsetSetInvalidKey() { $this->expectException(Exception::class); $this->expectExceptionMessage('Object is a dictionary, not a list'); From 5d1b45c9abcb8e9f8eebd1ed8243d4752fab4d90 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 22:55:36 +0200 Subject: [PATCH 3/9] HeadersTest: refactor array access tests * Merge two tests doing the exact same thing, into one test using a data provider with named data sets. * Add docblock documentation to the test and the data provider. --- tests/Response/HeadersTest.php | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index 9a0a87e39..d248f4672 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -16,18 +16,33 @@ public function testOffsetSetInvalidKey() { $headers[] = 'text/plain'; } - public function testArrayAccess() { + /** + * Test array access for the object is supported and supported in a case-insensitive manner. + * + * @dataProvider dataCaseInsensitiveArrayAccess + * + * @param string $key Key to request. + * + * @return void + */ + public function testCaseInsensitiveArrayAccess($key) { $headers = new Headers(); $headers['Content-Type'] = 'text/plain'; - $this->assertSame('text/plain', $headers['Content-Type']); + $this->assertSame('text/plain', $headers[$key]); } - public function testCaseInsensitiveArrayAccess() { - $headers = new Headers(); - $headers['Content-Type'] = 'text/plain'; - $this->assertSame('text/plain', $headers['CONTENT-TYPE']); - $this->assertSame('text/plain', $headers['content-type']); + /** + * Data provider. + * + * @return array + */ + public function dataCaseInsensitiveArrayAccess() { + return array( + 'access using case as set' => array('Content-Type'), + 'access using lowercase' => array('content-type'), + 'access using uppercase' => array('CONTENT-TYPE'), + ); } public function testMultipleHeaders() { From d91f9ccfa81cbd997d79f02f165b59710cfbfa86 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:06:00 +0200 Subject: [PATCH 4/9] HeadersTest::testIteration(): various minor tweaks * Add `$message` parameter to each assertion as there are multiple assertions in the test. * Make the exception more descriptive (not that I expect it to ever be thrown). * Add a docblock to the test. * Remove the `@depends` annotation which doesn't make any sense here. --- tests/Response/HeadersTest.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index d248f4672..1e05d7046 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -54,7 +54,12 @@ public function testMultipleHeaders() { } /** - * @depends testArrayAccess + * Test iterator access for the object is supported. + * + * Includes making sure that: + * - keys are handled case-insensitively. + * + * @return void */ public function testIteration() { $headers = new Headers(); @@ -64,13 +69,13 @@ public function testIteration() { foreach ($headers as $name => $value) { switch (strtolower($name)) { case 'content-type': - $this->assertSame('text/plain', $value); + $this->assertSame('text/plain', $value, 'Content-Type header does not match'); break; case 'content-length': - $this->assertSame('10', $value); + $this->assertSame('10', $value, 'Content-Length header does not match'); break; default: - throw new Exception('Invalid name: ' . $name); + throw new Exception('Invalid offset key: ' . $name); } } } From 30171ec6b1c2593e8c14bf9fd3c9a1c2972119dc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:08:24 +0200 Subject: [PATCH 5/9] HeadersTest::testIteration(): safeguard the flattening The `Header::getIterator()` method uses the `Header::flatten()` method, but there was no test safeguarding this functionality against regressions. Fixed now by adding an additional check to the `HeadersTest::testIteration()` method. --- tests/Response/HeadersTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index 1e05d7046..c1df98c4e 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -58,6 +58,7 @@ public function testMultipleHeaders() { * * Includes making sure that: * - keys are handled case-insensitively. + * - multiple keys with the same name are flattened into one value. * * @return void */ @@ -65,9 +66,14 @@ public function testIteration() { $headers = new Headers(); $headers['Content-Type'] = 'text/plain'; $headers['Content-Length'] = 10; + $headers['Accept'] = 'text/html;q=1.0'; + $headers['Accept'] = '*/*;q=0.1'; foreach ($headers as $name => $value) { switch (strtolower($name)) { + case 'accept': + $this->assertSame('text/html;q=1.0,*/*;q=0.1', $value, 'Accept header does not match'); + break; case 'content-type': $this->assertSame('text/plain', $value, 'Content-Type header does not match'); break; From 394d1e54b5070a10445221dca3203d5bff185e79 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:15:57 +0200 Subject: [PATCH 6/9] HeadersTest: add extra test for `offsetGet()` .. for functionality previously not covered by a test. --- tests/Response/HeadersTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index c1df98c4e..de29eaca8 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -53,6 +53,18 @@ public function testMultipleHeaders() { $this->assertSame('text/html;q=1.0,*/*;q=0.1', $headers['Accept']); } + /** + * Test that null is returned when a non-registered header is requested. + * + * @return void + */ + public function testOffsetGetReturnsNullForNonRegisteredHeader() { + $headers = new Headers(); + $headers['Content-Type'] = 'text/plain'; + + $this->assertNull($headers['not-content-type']); + } + /** * Test iterator access for the object is supported. * From ed2df1f6afaa39977ccf811cbc8a3a0a23aa631f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:37:47 +0200 Subject: [PATCH 7/9] HeadersTest: add test for `getValues()` ... which was, so far, not covered by tests yet. --- tests/Response/HeadersTest.php | 57 ++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index de29eaca8..e2b9296a8 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -65,6 +65,63 @@ public function testOffsetGetReturnsNullForNonRegisteredHeader() { $this->assertNull($headers['not-content-type']); } + /** + * Test retrieving all values for a given header (case-insensitively). + * + * @dataProvider dataGetValues + * + * @param string $key Key to request. + * @param string|null $expected Expected return value. + * + * @return void + */ + public function testGetValues($key, $expected) { + $headers = new Headers(); + $headers['Content-Type'] = 'text/plain'; + $headers['Content-Length'] = 10; + $headers['Accept'] = 'text/html;q=1.0'; + $headers['Accept'] = '*/*;q=0.1'; + + $this->assertSame($expected, $headers->getValues($key)); + } + + /** + * Data provider. + * + * @return array + */ + public function dataGetValues() { + return array( + 'using case as set, single entry header' => array( + 'key' => 'Content-Type', + 'expected' => array( + 'text/plain', + ), + ), + 'using lowercase, single entry header' => array( + 'key' => 'content-length', + 'expected' => array( + 10, + ), + ), + 'using uppercase, multiple entry header' => array( + 'key' => 'ACCEPT', + 'expected' => array( + 'text/html;q=1.0', + '*/*;q=0.1', + ), + ), + 'non-registered string key' => array( + 'key' => 'my-custom-header', + 'expected' => null, + ), + 'non-registered integer key' => array( + 'key' => 10, + 'expected' => null, + ), + ); + } + /** * Test iterator access for the object is supported. * From f09113c88f208f3f3f3970b10acc7184a44ca1bd Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:43:56 +0200 Subject: [PATCH 8/9] HeadersTest: add docblocks to the rest of the tests --- tests/Response/HeadersTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index e2b9296a8..8b7d37ee4 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -8,6 +8,11 @@ final class HeadersTest extends TestCase { + /** + * Test receiving an Exception when no key is provided when setting an entry. + * + * @return void + */ public function testOffsetSetInvalidKey() { $this->expectException(Exception::class); $this->expectExceptionMessage('Object is a dictionary, not a list'); @@ -45,6 +50,12 @@ public function dataCaseInsensitiveArrayAccess() { ); } + /** + * Test that when multiple headers are set using the same key, requesting the key will return the + * combined values flattened into a single, comma-separated string. + * + * @return void + */ public function testMultipleHeaders() { $headers = new Headers(); $headers['Accept'] = 'text/html;q=1.0'; From f87531b01fd2d631b614cdcadbe31009480030f8 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 2 Sep 2021 23:44:02 +0200 Subject: [PATCH 9/9] HeadersTest: add `@covers` tags --- tests/Response/HeadersTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/Response/HeadersTest.php b/tests/Response/HeadersTest.php index 8b7d37ee4..7e013448d 100644 --- a/tests/Response/HeadersTest.php +++ b/tests/Response/HeadersTest.php @@ -6,11 +6,16 @@ use WpOrg\Requests\Response\Headers; use WpOrg\Requests\Tests\TestCase; +/** + * @coversDefaultClass \WpOrg\Requests\Response\Headers + */ final class HeadersTest extends TestCase { /** * Test receiving an Exception when no key is provided when setting an entry. * + * @covers ::offsetSet + * * @return void */ public function testOffsetSetInvalidKey() { @@ -24,6 +29,9 @@ public function testOffsetSetInvalidKey() { /** * Test array access for the object is supported and supported in a case-insensitive manner. * + * @covers ::offsetSet + * @covers ::offsetGet + * * @dataProvider dataCaseInsensitiveArrayAccess * * @param string $key Key to request. @@ -54,6 +62,10 @@ public function dataCaseInsensitiveArrayAccess() { * Test that when multiple headers are set using the same key, requesting the key will return the * combined values flattened into a single, comma-separated string. * + * @covers ::offsetSet + * @covers ::offsetGet + * @covers ::flatten + * * @return void */ public function testMultipleHeaders() { @@ -67,6 +79,8 @@ public function testMultipleHeaders() { /** * Test that null is returned when a non-registered header is requested. * + * @covers ::offsetGet + * * @return void */ public function testOffsetGetReturnsNullForNonRegisteredHeader() { @@ -79,6 +93,8 @@ public function testOffsetGetReturnsNullForNonRegisteredHeader() { /** * Test retrieving all values for a given header (case-insensitively). * + * @covers ::getValues + * * @dataProvider dataGetValues * * @param string $key Key to request. @@ -140,6 +156,9 @@ public function dataGetValues() { * - keys are handled case-insensitively. * - multiple keys with the same name are flattened into one value. * + * @covers ::getIterator + * @covers ::flatten + * * @return void */ public function testIteration() {