Problem/Motivation

TaxonomyFieldTidTest makes no HTTP requests but is a functional test.

Proposed resolution

Convert it to a kernel test.

Comments

jian he created an issue. See original summary.

jian he’s picture

Status: Active » Needs review
StatusFileSize
new3.35 KB
jian he’s picture

StatusFileSize
new3.36 KB
new593 bytes

Fix assertEqual

claudiu.cristea’s picture

Status: Needs review » Needs work

@jian he, thank you for the conversion.

  1. +++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
    @@ -0,0 +1,69 @@
    +  /**
    +   * Modules to enable.
    +   *
    +   * @var array
    +   */
    ...
    +  /**
    +   * Views used by this test.
    +   *
    +   * @var array
    +   */
    

    Docblocks should be replaced with {@inheritdoc}.

  2. +++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
    @@ -0,0 +1,69 @@
    +   * @var \Drupal\taxonomy\Entity\Term
    

    s/Term/TermInterface

  3. +++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
    @@ -0,0 +1,69 @@
    +  protected function setUp($import_test_views = TRUE) {
    

    Missed docblock. Should {@inheritdoc}

  4. +++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
    @@ -0,0 +1,69 @@
    +  public function testViewsHandlerTidField() {
    

    Missing documentation block.

  5. +++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
    @@ -0,0 +1,69 @@
    +    $expected = \Drupal::l($this->term1->label(), $this->term1->toUrl());
    

    Uses Drupal::l(), which is deprecated. Should be updated.

claudiu.cristea’s picture

+++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
@@ -0,0 +1,69 @@
+  public static $modules = ['taxonomy', 'taxonomy_test_views', 'text', 'filter'];

s/public/protected

jian he’s picture

Status: Needs work » Needs review
StatusFileSize
new3.42 KB
new2.16 KB

@claudiu.cristea, thanks for the review.
This patch fixed all #4 and #5.

Status: Needs review » Needs work

The last submitted patch, 6: 3042640-TaxonomyFieldTidTest-6.patch, failed testing. View results

claudiu.cristea’s picture

+++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
@@ -18,26 +19,25 @@ class TaxonomyFieldTidTest extends ViewsKernelTestBase {
-  public static $modules = ['taxonomy', 'taxonomy_test_views', 'text', 'filter'];
+  protected static $modules = ['taxonomy', 'taxonomy_test_views', 'text', 'filter'];

Yes, this was a bad idea. I didn't realize we're extending from ViewsKernelTestBase. Sorry.

+++ b/core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyFieldTidTest.php
@@ -0,0 +1,72 @@
+    $this->assertEquals($expected, $actual);

Shouldn't this be $expected->toString()?

jian he’s picture

Status: Needs work » Needs review
StatusFileSize
new3.42 KB
new650 bytes

#8.1 oh, I does not realized that ViewsKernelTestBase use public $module.
#8.2 does not need toString(). Both $expected and $actual are array, and the assertEquals() can compare the two array.

Status: Needs review » Needs work

The last submitted patch, 9: 3042640-TaxonomyFieldTidTest-9.patch, failed testing. View results

jian he’s picture

Status: Needs work » Needs review
StatusFileSize
new3.43 KB
new616 bytes

ah, #8.2 is right

claudiu.cristea’s picture

Status: Needs review » Reviewed & tested by the community

Thank you.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 603f0d4cc7 to 8.8.x and 88ecbd912d to 8.7.x. Thanks!

As a test only change backported to 8.7.x - I discussed backporting test-only changes with @catch (as a release manager).

  • alexpott committed 603f0d4 on 8.8.x
    Issue #3042640 by jian he, claudiu.cristea: Convert TaxonomyFieldTidTest...

  • alexpott committed 88ecbd9 on 8.7.x
    Issue #3042640 by jian he, claudiu.cristea: Convert TaxonomyFieldTidTest...

Status: Fixed » Closed (fixed)

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