#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.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
xjm’s picture

Issue tags: +Needs tests
herom’s picture

issue summary updated.

damiankloip’s picture

Status: Active » Needs review
FileSize
4.15 KB

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.

herom’s picture

@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.

dawehner’s picture

Issue tags: -Needs tests
FileSize
7.12 KB
8.49 KB

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).

damiankloip’s picture

  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!

dawehner’s picture

FileSize
8.06 KB

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".

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community

This coverage looks great now.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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

Anonymous’s picture

Issue summary: View changes

update issue summary