Sub issue of #2014671: [META] Convert all field types to plugins.

Looks like poor field_test.module got forgotten, so here's a start. Have moved a few things over, validation isn't adjusted yet and some crazy parts with that memorize stuff aren't either. I think it makes sense to re-think that a bit. Looks like basic crud tests worked, let's see about the others.

Related change notice https://drupal.org/node/2064123

Files: 
CommentFileSizeAuthor
#41 interdiff.txt564 bytesandypost
#41 field-test-type-2083803-41.patch27.12 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,840 pass(es).
[ View ]
#39 interdiff.txt982 bytesandypost
#39 field-test-type-2083803-39.patch27.23 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,915 pass(es).
[ View ]
#38 interdiff.txt2.06 KBandypost
#38 field-test-type-2083803-38.patch27.23 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,452 pass(es).
[ View ]
#35 interdiff.txt4.23 KBandypost
#35 field-test-type-2083803-35.patch27.02 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,835 pass(es).
[ View ]
#33 field-test-type-2083803-33.patch24.57 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 58,766 pass(es).
[ View ]
#33 field-test-type-2083803-33-interdiff.txt786 bytesBerdir
#31 field-test-type-2083803-31.patch24.56 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 58,836 pass(es).
[ View ]
#31 field-test-type-2083803-31-interdiff.txt1.92 KBBerdir
#28 field-test-type-2083803-28.patch24.51 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 58,307 pass(es), 11 fail(s), and 0 exception(s).
[ View ]
#20 drupal8.field-system.2083803-20.patch24.49 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.field-system.2083803-20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 interdiff.txt2.6 KBandypost
#13 drupal8.field-system.2083803-13.patch24.49 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,730 pass(es).
[ View ]
#8 interdiff.txt3.89 KBandypost
#8 drupal8.field-system.2083803-8.patch24.31 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 59,228 pass(es).
[ View ]
#8 drupal8.field-system.2083803-noinstall-8.patch24.47 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 58,858 pass(es).
[ View ]
#6 interdiff.txt3.2 KBandypost
#6 drupal8.field-system.2083803-6-noinstallfile.patch27.2 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#6 drupal8.field-system.2083803-6.patch25.22 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 59,353 pass(es), 3 fail(s), and 3 exception(s).
[ View ]
#5 interdiff.txt4.1 KBandypost
#5 drupal8.field-system.2083803-5.patch27.56 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 59,251 pass(es), 50 fail(s), and 34 exception(s).
[ View ]
#4 interdiff.txt3.87 KBandypost
#4 drupal8.field-system.2083803-4.patch24.3 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#3 interdiff.txt12.49 KBandypost
#3 drupal8.field-system.2083803-3.patch25.58 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 59,328 pass(es), 49 fail(s), and 35 exception(s).
[ View ]
#1 field-test-types-2083803-1.patch18.54 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 58,362 pass(es), 86 fail(s), and 51 exception(s).
[ View ]

Comments

StatusFileSize
new18.54 KB
FAILED: [[SimpleTest]]: [MySQL] 58,362 pass(es), 86 fail(s), and 51 exception(s).
[ View ]

Here's a first patch.

Status:Needs review» Needs work

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

Status:Needs work» Needs review
StatusFileSize
new25.58 KB
FAILED: [[SimpleTest]]: [MySQL] 59,328 pass(es), 49 fail(s), and 35 exception(s).
[ View ]
new12.49 KB

Filed separate issue to fix broken test #2087449: Fix broken tests FieldAttachOtherTest->testFieldAttachCache()
Fixed tests: ShapeItemTest, FieldValidationTest

StatusFileSize
new24.3 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new3.87 KB

Reverted back changed test about 'default_value_function' - that's needed
Removed hook_field_load - prepareCache() already there

StatusFileSize
new27.56 KB
FAILED: [[SimpleTest]]: [MySQL] 59,251 pass(es), 50 fail(s), and 34 exception(s).
[ View ]
new4.1 KB

Fixed FieldInfoTest

StatusFileSize
new25.22 KB
FAILED: [[SimpleTest]]: [MySQL] 59,353 pass(es), 3 fail(s), and 3 exception(s).
[ View ]
new27.2 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new3.2 KB

All tests are passed locally.
1) FieldAttachOtherTest.php changed according #2087449: Fix broken tests FieldAttachOtherTest->testFieldAttachCache()
2) field_test_widget_multiple_validate moved to public static method TestFieldWidgetMultiple::multipleValidate()

PS drupal8.field-system.2083803-6-noinstallfile.patch test the need in install file

Status:Needs review» Needs work

The last submitted patch, drupal8.field-system.2083803-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new24.47 KB
PASSED: [[SimpleTest]]: [MySQL] 58,858 pass(es).
[ View ]
new24.31 KB
PASSED: [[SimpleTest]]: [MySQL] 59,228 pass(es).
[ View ]
new3.89 KB

A bit more clean-up

Status:Needs review» Needs work

+++ b/core/modules/field/lib/Drupal/field/Tests/FieldInfoTest.php
@@ -327,4 +327,50 @@ function testWidgetDefinition() {
+  /**
+   * Returns field info definition.
+   */
+  protected function field_test_field_info() {

Why don't you get the field plugin definition like everyone else and duplicate all the data here? :)

Suppose better to rename this to expectedFieldInfo()

+++ b/core/modules/field/lib/Drupal/field/Tests/FieldInfoTest.php
@@ -23,7 +23,7 @@ public static function getInfo() {
+    $field_test_info = $this->field_test_field_info();
     $info = \Drupal::service('plugin.manager.entity.field.field_type')->getDefinitions();
     foreach ($field_test_info as $t_key => $field_type) {
@@ -283,7 +283,7 @@ function testFieldMap() {
+    $info = $this->field_test_field_info();
     foreach ($info as $type => $data) {
@@ -327,4 +327,50 @@ function testWidgetDefinition() {
+  protected function field_test_field_info() {
+    return array(
+      'test_field' => array(

because this config used to validate result of getDefinition()

  1. +++ b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/field/field_type/TestItem.php
    @@ -0,0 +1,153 @@
    +  public function delete() {
    +    // In D7 it was hook_field_delete().
    +    field_test_memorize('field_test_field_delete', array($this->getEntity()));
    +  }

    I don't think we need to document how it was called in 7.x..

  2. +++ b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/field/field_type/TestItem.php
    @@ -0,0 +1,153 @@
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public function isEmpty() {
    +    $value = $this->get('value')->getValue();
    +    return empty($value);
    +  }

    Can be written as return empty($this->value).

About method name, I'd suggest getExpectedFieldTypeDefinition() or something similar with getExpected..()

Status:Needs work» Needs review
StatusFileSize
new24.49 KB
PASSED: [[SimpleTest]]: [MySQL] 58,730 pass(es).
[ View ]
new2.6 KB

addressed

Status:Needs review» Needs work
Issue tags:-Field API

The last submitted patch, drupal8.field-system.2083803-13.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Field API

Interdiff looks fine to me, so +1 from my side. A bit surprised that we apparently don't have any tests remaining that required the memorize stuff for load/insert/update, but fine with me if that went out somewhere.

Wondering if we should add something else to the list of arguments we put in there, it used to contain the items too, so maybe add $this or $this->getValue() (easier to serialize) to it?

@yched?

I'd prefer to keep conversion issues in the scope, so filed follow-up #2089369: Extend tests for field hooks

Not saying we need more tests.

I'm just saying that I *thought* we had more tests for this and I was confused to not see more test fails. But fine with me, pretty sure we have enough implicit test coverage of those methods. If anything, we should write a unit test for this kind of test when we can mock everything we need to at some point.

StatusFileSize
new24.49 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.field-system.2083803-20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

@yched we needs your approval!
merge HEAD

Issue tags:-Field API

Status:Needs review» Needs work

The last submitted patch, drupal8.field-system.2083803-20.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Field API

patch still applies

Issue tags:-Field API

Status:Needs review» Needs work
Issue tags:+Field API

The last submitted patch, drupal8.field-system.2083803-20.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new24.51 KB
FAILED: [[SimpleTest]]: [MySQL] 58,307 pass(es), 11 fail(s), and 0 exception(s).
[ View ]

Re-roll.

Status:Needs review» Reviewed & tested by the community

Looks good, another critical down if green!

Status:Reviewed & tested by the community» Needs work

The last submitted patch, field-test-type-2083803-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.92 KB
new24.56 KB
PASSED: [[SimpleTest]]: [MySQL] 58,836 pass(es).
[ View ]

This should fix the test fails.

+++ b/core/modules/field/tests/modules/field_test/lib/Drupal/field_test/Plugin/Validation/Constraint/TestFieldConstraint.php
@@ -0,0 +1,40 @@
+ * Contains \Drupal\field_test\Plugin\Validation\Constraint\TestField.

Nitpick, should probably be \TestFieldConstraint no ? Or do we do this different for constraint classes ?

StatusFileSize
new786 bytes
new24.57 KB
PASSED: [[SimpleTest]]: [MySQL] 58,766 pass(es).
[ View ]

Fixed that. No, constraint classes are not that special :)

Status:Needs review» Reviewed & tested by the community

Great!

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new27.02 KB
PASSED: [[SimpleTest]]: [MySQL] 58,835 pass(es).
[ View ]
new4.23 KB

I think the removed hook_field_load should not be referenced
Also there's no need in .install file

+++ /dev/null
@@ -1,63 +0,0 @@
-/**
- * Implements hook_install().
- */
-function field_test_install() {
-  // hook_entity_info_alter() needs to be executed as last.
-  module_set_weight('field_test', 1);
-}

I explicitly didn't remove the .install file because it has this hunk in there, which (even if not necessary anymore, not sure), has nothing to do with this and shouldn't be removed together with it?

Also not sure why the getValue() change is necessary? But then !$this->isEmpty() would make more sense I think and the test should probably reference the fully namespaced classname.

StatusFileSize
new27.23 KB
PASSED: [[SimpleTest]]: [MySQL] 58,452 pass(es).
[ View ]
new2.06 KB

Fixed to full namespace.

+++ /dev/null
@@ -1,63 +0,0 @@
-function field_test_install() {
-  // hook_entity_info_alter() needs to be executed as last.
-  module_set_weight('field_test', 1);
-}

Was added in #565480: TF #2: Multilingual field handling and no more needed because field translation is changed

StatusFileSize
new27.23 KB
PASSED: [[SimpleTest]]: [MySQL] 58,915 pass(es).
[ View ]
new982 bytes

And missed isEmpty()

@install stuff: Ok, it's quite possible that it's no longer used, but it's not related to this issue and it only removes 50% of it, there's still the actual hook implementation. I'd suggest to not touch that here :)

StatusFileSize
new27.12 KB
PASSED: [[SimpleTest]]: [MySQL] 58,840 pass(es).
[ View ]
new564 bytes

Status:Needs review» Reviewed & tested by the community

Ok, let's get this in now, good catches andypost!

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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

Issue summary:View changes

Updated issue summary.