Problem/Motivation

Patches that add new kernel tests to 8.0.x (e.g. #2644088: DefaultTableMapping::getFieldTableName does not report table for fields with dedicated storage) should be able to use the same EntityKernelTestBase base test class that was committed only to 8.1.x and 8.2.x.

Proposed resolution

Backport the test class to 8.0.x.

Remaining tasks

None.

User interface changes

Nope.

API changes

Api addition: a new base test class is available for entity unit tests based on PHPUnit.

Data model changes

Nope.

CommentFileSizeAuthor
#2 2683391.patch3.72 KBamateescu

Comments

amateescu created an issue. See original summary.

amateescu’s picture

amateescu commented
Status: Active » Needs review
StatusFileSize
new 2683391.patch3.72 KB

Let's try this.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

When discussing with alexpott, @dawehner and I discussed that it shouldn't be needed to backport the other issue.

I think it's enough if contrib tests work with the current and upcoming minor version, so once 8.1.x is out, you could convert to the new one.

But I'm also OK if someone thinks this is important enough. Just the base class is certainly already better and less disruption.

amateescu’s picture

amateescu commented

IMO the disruption is 0 and it lets patches like #2644088: DefaultTableMapping::getFieldTableName does not report table for fields with dedicated storage be in sync for all three branches.

kevin.dutra’s picture

kevin.dutra commented
Status: Needs review » Needs work

A little minor code style cleanup:

+  /**
+   * {@inheritdoc}
+   */
+  public static function assertEquals($expected, $actual, $message = '', $delta = 0.0, $maxDepth = 10, $canonicalize = false, $ignoreCase = false) {
+    $expected = static::castSafeStrings($expected);
+    $actual = static::castSafeStrings($actual);
+    parent::assertEquals($expected, $actual, $message, $delta, $maxDepth, $canonicalize, $ignoreCase);
+  }
+
+
  1. FALSE should be in all caps.
  2. $maxDepth and $ignoreCase should be lowercase with underscores.
  3. There's an extra newline at the end.
amateescu’s picture

amateescu commented
Status: Needs work » Needs review

This is just a part of the patch that was committed in #2679096: Convert Kernel Tests in Drupal\system\Tests\Entity to phpunit. I think the arguments were not changed to fit our coding standards in order to be consistent with the parent method.

kevin.dutra’s picture

kevin.dutra commented
Status: Needs review » Reviewed & tested by the community

Ah, okay -- I guess I'll defer to the original issue on that. Since this change is pretty minor, low impact, and helps to avoid reworking patches among the different 8.x branches, I'm going to go ahead and mark this RTBC.

  • catch committed bce8913 on 8.0.x 
    Issue #2683391 by amateescu: Backport EntityKernelTestBase to 8.0.x
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed 
+++ b/core/modules/simpletest/src/AssertHelperTrait.php
@@ -23,7 +23,7 @@
+  protected static function castSafeStrings($value) {

I had to double check on 3v4l.org that callers of this won't get a message, but it's only the other way 'round that you get E_DEPRECATED if you call it the wrong way.

Committed/pushed to 8.0.x, thanks!

Status: Fixed » Closed (fixed)

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