Not alot of explaining needed, convert this test from a DrupalUnitTest to a PHPUnit test. I think it makes sense to use data providers too.

CommentFileSizeAuthor
#3 2023095-3.patch7.47 KBdamiankloip
#3 interdiff-2023095-3.txt1.54 KBdamiankloip
#1 2023095-1.patch6.84 KBdamiankloip
#1 interdiff-2023095-1.txt1.8 KBdamiankloip
vdc-pluginBaseTest-PHPUnit.patch6.84 KBdamiankloip

Comments

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff-2023095-1.txt1.8 KB
new 2023095-1.patch6.84 KB

I spoke to dawehner on IRC, the provider methods should be called provider* and not *provider.

dawehner’s picture

dawehner
Primary language German
commented

Really nice! Here are just two nitpicks.

+++ b/core/modules/views/tests/Drupal/views/Tests/PluginBaseTest.phpundefined
@@ -2,37 +2,72 @@
+  protected function setUp() {

Let's inheritdoc the setup method.

+++ b/core/modules/views/tests/Drupal/views/Tests/PluginBaseTest.phpundefined
@@ -2,37 +2,72 @@
+  public function testUnpackOptions($storage, $options, $definition, $expected, $all = FALSE) {
...
+  public function testSetOptionDefault($storage, $definition, $expected) {

It would be cool to document these methods.

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff-2023095-3.txt1.54 KB
new 2023095-3.patch7.47 KB

Thanks! added that docblock and docs.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Thank you

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 893ae86 and pushed to 8.x. Thanks!

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/views/tests/Drupal/views/Tests/PluginBaseTest.phpundefined
@@ -2,37 +2,93 @@
+    $this-> testHelperPlugin = new TestHelperPlugin(array(), 'default', array());

needs follow-up to remove space after $this->

Status: Fixed » Closed (fixed)

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

bdone’s picture

bdone commented

#2046845: remove space after $this-> testHelperPlugin adds follow-up to remove trailing space in PluginBaseTest