#1754216: Meta: Add one test that tests each plugin type intensive

Most of the logic in PagerPluginBase is already covered in PagerTest.php: function testPagerApi(). But the tests are written against the Full pager. So, the task here is to:

  1. write a minimum test pager type that extends PagerPluginBase,
    in views/tests/modules/views_test_data/lib/Drupal/views_test_data/Plugin/views
  2. add a new view based on this pager type,
    in views/tests/modules/views_test_config/test_views
  3. convert testPagerApi to use this new view, instead of 'test_pager_full'
  4. add test to cover function hasMoreRecords() in PagerPluginBase.
Files: 
CommentFileSizeAuthor
#8 vdc-1754254-8.patch8.06 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,866 pass(es).
[ View ]
#6 vdc-1754254-6.patch8.49 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,920 pass(es).
[ View ]
#6 interdiff.txt7.12 KBdawehner
#4 1754254.patch4.15 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,792 pass(es).
[ View ]

Comments

Project:Views» Drupal core
Version:8.x-3.x-dev» 8.x-dev
Component:Code» views.module

Issue tags:+Needs tests

issue summary updated.

Status:Active» Needs review
StatusFileSize
new4.15 KB
PASSED: [[SimpleTest]]: [MySQL] 58,792 pass(es).
[ View ]

We should actually be able to test this OK with PHPUnit. Using the mocking, we can get a mock for the abstract PagerPluginBase class and test all the methods.

Then we have a proper test for just testing this base functionality :) what do you think?

I have not got time to finish this right now, so if anyones else so desires, be my guest... I've left a todo in the test class.

@damiankloip I wrote the current summary based on the other issues in the Meta issue.
The PHPUnit looks fine too, I guess. but in that case, the testPagerApi function in PagerTest should be removed, because this is just duplicating that test.

Issue tags:-Needs tests
StatusFileSize
new7.12 KB
new8.49 KB
PASSED: [[SimpleTest]]: [MySQL] 58,920 pass(es).
[ View ]

Great test coverage so far!

Wow, mocking the dbtng objects isn't straighforward.
@herom
I don't see where PagerTest::testPagerApi directly tests these functions (note: the call through the executable is a worthwile test).

  1. +++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/pager/PagerPluginBaseTest.php
    @@ -0,0 +1,266 @@
    +   * @see \Drupal\views\Plugin\views\pager\PagerPluginBase::getItemsPerPage()

    Good docs , but this seems a bit overkill to me. We have an @see on the class to the PagerPluginBase class, then the 'Tests the .... method' part, then an @see too!

  2. +++ b/core/phpunit.xml.dist
    @@ -1,6 +1,6 @@
    -<phpunit bootstrap="tests/bootstrap.php" colors="true" strict="true">
    +<phpunit bootstrap="tests/bootstrap.php" colors="true">

    Did that change get reverted already?

Otherwise, nice additions!

StatusFileSize
new8.06 KB
PASSED: [[SimpleTest]]: [MySQL] 58,866 pass(es).
[ View ]

Good docs , but this seems a bit overkill to me. We have an @see on the class to the PagerPluginBase class, then the 'Tests the .... method' part, then an @see too!

The reason why I do like this is because I can just directly jump to the code.

Removed the phpunit "hack".

Status:Needs review» Reviewed & tested by the community

This coverage looks great now.

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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

Issue summary:View changes

update issue summary