Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
3.59 KB

No test coverage yet, not really sure how much use that actually is. I't just a copy & paste of other tests.

What might be interesting would be to test shape as that does not use ->value but shape and color.

Status: Needs review » Needs work

The last submitted patch, field-test-1839082-1.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review

#1: field-test-1839082-1.patch queued for re-testing.

fago’s picture

+++ b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Type/ShapeItem.php
@@ -0,0 +1,44 @@
+   * @see self::getPropertyDefinitions()

This should reference the class now, for both field types. Else it looks good to me, so let's add basic test coverages and move on?

Berdir’s picture

FileSize
14.1 KB

Added test for both field types, including a base test class for all field type item tests with a faster setup (less full module installs) and converted existing tests to that base class.

fago’s picture

Status: Needs review » Needs work

Great work on the tests!

+++ b/core/modules/field/lib/Drupal/field/Tests/ShapeItemTest.php
@@ -0,0 +1,91 @@
+ * Tests the new entity API for the field field type.

shape field?

+++ b/core/modules/field/lib/Drupal/field/Tests/TestItemTest.php
@@ -0,0 +1,83 @@
+ * Tests the new entity API for the field field type.

again, field field.

+++ b/core/modules/field/tests/modules/field_test/field_test.entity.inc
@@ -172,6 +172,10 @@ function field_test_create_entity($id = 1, $vid = 1, $bundle = 'test_bundle', $l
 function field_test_entity_test_load($ftid, $ftvid = NULL) {
+  // Prevent this from being called as hook_entity_test_load().

ouch. Can we rename the function instead?

Berdir’s picture

Will re-roll for the other two changes. I'd like to keep changes to the test_entity related code as minimal as possible. I am working on completely replacing it over in #1822000: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest. The function is references quite a bit from other files that I would have to change as well.

Berdir’s picture

Status: Needs work » Needs review
FileSize
1.04 KB
14.1 KB

Fixed the comments.

fago’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, new comments read good.

Will re-roll for the other two changes. I'd like to keep changes to the test_entity related code as minimal as possible. I am working on completely replacing it over in #1822000: Clean up Drupal\field_test\Plugin\Entity\Type\TestEntity. The function is references quite a bit from other files that I would have to change as well.

Sounds good!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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