docs: replace type in Pager

[ddevsr]

Description
See #6310

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

What is this test testing?

[ddevsr]

For covered testing if $returnObject = true

$this->pager->store('bar', 5, 25, 100, 1);
$uri = $this->pager->getPageURI(7, 'bar', true);

[kenjis]

Why don't you assert that $uri is instance of URI?

[ddevsr]

I call getTotalSegments and getSegment because part of the function URI class. Its okay if only assert instance of URI

[kenjis]

Please try to write a test that is as clear as possible what you are trying to test.

And if you want to test two aspects, you can add two test methods.

[ddevsr]

What about this?

--- a/tests/system/Pager/PagerTest.php
+++ b/tests/system/Pager/PagerTest.php
@@ -184,11 +184,28 @@ final class PagerTest extends CIUnitTestCase

     public function testGetPageURIWithURIReturnObject()
     {
-        $this->pager->store('bar', 5, 25, 100, 1);
+        $_GET['page'] = 3;
+        $_GET['foo']  = 'bar';
+
+        $this->pager->store('default', 3, 25, 100, 1);

-        $uri = $this->pager->getPageURI(7, 'bar', true);
+        $uri = $this->pager->getPageURI(5, 'default', true);

         $this->assertInstanceOf(URI::class, $uri);
+        $this->assertSame(
+            'page=3&foo=bar',
+            $uri->getQuery()
+        );
+
+        $this->pager->store('new', 3, 25, 100, 1);
+
+        $uriOnly = $this->pager->only(['foo'])->getPageURI(7, 'new', true);
+
+        $this->assertInstanceOf(URI::class, $uriOnly);
+        $this->assertSame(
+            'foo=bar',
+            $uriOnly->getQuery()
+        );
     }

[kenjis]

It is better than the first test.
But there are many test aspects:

• return URI object
• query parameters in URI object
• only() method

It is better to write one test for one aspect.

If the test method is testGetPageURIWithURIReturnObject, the current test is enough.
If you want to write a test for only() or query parameters, please add another test method.
Or is there any test already?

[ddevsr]

getPageURI only 2 aspect, with only or without only. And this topic coverage getPageURI return URI object

    public function testGetPageURIWithURIReturnObject()
    {
        $_GET['page'] = 3;
        $_GET['foo']  = 'bar';

        $this->pager->store('default', 5, 25, 100, 1);

        $uri = $this->pager->getPageURI(5, 'default', true);

        $this->assertInstanceOf(URI::class, $uri);
        $this->assertSame(
            'page=3&foo=bar',
            $uri->getQuery()
        );
    }

    public function testOnlyGetPageURIWithURIReturnObject()
    {
        $_GET['page'] = 3;
        $_GET['foo']  = 'bar';

        $this->pager->store('default', 5, 25, 100, 1);

        $uriOnly = $this->pager->only(['foo'])->getPageURI(5, 'default', true);

        $this->assertInstanceOf(URI::class, $uriOnly);
        $this->assertSame(
            'foo=bar',
            $uriOnly->getQuery()
        );
    }

[kenjis]

$this->pager->store('default', 5, 25, 100, 1);
The last parameter means the first URI segment is the page number.

$_GET['page'] = 3; means it is a query string that is not the page number, but the name is page accidentally.

I think these tests are testing too tricky situations.

If you want to test only() or query string, I think it is better to use the cases with the page number query string ,
not using URI segment.

[kenjis]

@ddevsr It seems you did git merge, but if you can use git rebase, please use it in stead of git merge
when you update your PR branch.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch

[MGatner]

@kenjis has gone through this thoroughly so I just checked out the end result. Very nice QA, thank you!