Let's speed up the Field API tests and use DrupalUnitTestBase where possible. Patch upcoming.

CommentFileSizeAuthor
#16 1932382-followup.patch949 bytesamateescu
#11 field-dubt-1932382-11.patch41.51 KBBerdir
#11 field-dubt-1932382-11-interdiff.txt813 bytesBerdir
#9 field-dubt-1932382-9.patch41.56 KBBerdir
#9 field-dubt-1932382-9-interdiff.txt699 bytesBerdir
#7 field-dubt-1932382-7.patch42.24 KBBerdir
#4 field-dubt-1932382-4.patch41.17 KBBerdir
#1 1932382-1.patch4.29 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Active » Needs review
FileSize
1932382-1.patch4.29 KB

4 tests could be converted so far.

Status: Needs review » Needs work

The last submitted patch, 1932382-1.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Hah, I just had the same idea :)

Converted most of them already, will upload a patch in a few minutes.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
Issue tags: +Test suite performance
FileSize
field-dubt-1932382-4.patch41.17 KB

Notes:

- Patch is not based on swentel's patch, created mine in parallel today :)
- Also added a base class, additionally extended from EntityUnitBaseTest (will be renamed in the follow-up patch in #1893108: Convert most entity tests to DrupalUnitTestBase, so will need a re-roll then). Not 100% sure if that is necessary, shouldn't be too much to duplicate and we should probably avoid having too many dependencies in test classes.
- Killed FieldAttachTestBase and FieldItemUnitTestBase in favor of the new base class.
- Added assertRaw/Text helpers for various tests, there are a lot of those calls that should actually be a assertContains(), which we should probably add to TestBase but this allows to keep the impact of this change as small as possible.
- Moved a single test method from CrudTest and TranslationTest to separate test classes, so that the others can become DUBT.

Speed difference is quite impressive because there are a lot of test methods, will post a comparison later when I'm on my desktop.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Before:

$ php core/scripts/run-tests.sh --url http://d8/ "Field API"

Drupal test run
---------------

Tests to be run:
 - Field bulk delete tests (Drupal\field\Tests\BulkDeleteTest)
 - Field CRUD tests (Drupal\field\Tests\CrudTest)
 - Field Display API tests (Drupal\field\Tests\DisplayApiTest)
 - Field access tests (Drupal\field\Tests\FieldAccessTest)
 - Field attach tests (other) (Drupal\field\Tests\FieldAttachOtherTest)
 - Field attach tests (storage-related) (Drupal\field\Tests\FieldAttachStorageTest)
 - Field info tests (Drupal\field\Tests\FieldInfoTest)
 - Field instance CRUD tests (Drupal\field\Tests\FieldInstanceCrudTest)
 - Field form tests (Drupal\field\Tests\FormTest)
 - Field translations tests (Drupal\field\Tests\TranslationTest)
 - Field SQL Storage tests (Drupal\field_sql_storage\Tests\FieldSqlStorageTest)
 - Multilingual fields (Drupal\node\Tests\NodeFieldMultilingualTestCase)

Test run started:
 Sunday, March 3, 2013 - 21:54

Test summary
------------

Field bulk delete tests 75 passes, 0 fails, and 0 exceptions
Field CRUD tests 75 passes, 0 fails, and 0 exceptions
Field Display API tests 46 passes, 0 fails, and 0 exceptions
Field access tests 21 passes, 0 fails, 0 exceptions, and 5 debug messages
Field attach tests (other) 103 passes, 0 fails, and 0 exceptions
Field attach tests (storage-related) 94 passes, 0 fails, and 0 exceptions
Field info tests 73 passes, 0 fails, and 0 exceptions
Field instance CRUD tests 28 passes, 0 fails, and 0 exceptions
Field form tests 281 passes, 0 fails, 0 exceptions, and 63 debug messages
Field translations tests 100 passes, 0 fails, 0 exceptions, and 4 debug messages
Field SQL Storage tests 92 passes, 0 fails, and 0 exceptions
Multilingual fields 64 passes, 0 fails, 0 exceptions, and 21 debug messages

Test run duration: 3 min 11 sec

With patch

$ php core/scripts/run-tests.sh --url http://d8/ "Field API"

Drupal test run
---------------

Tests to be run:
 - Field active test (Drupal\field\Tests\ActiveTest)
 - Field bulk delete tests (Drupal\field\Tests\BulkDeleteTest)
 - Field CRUD tests (Drupal\field\Tests\CrudTest)
 - Field Display API tests (Drupal\field\Tests\DisplayApiTest)
 - Field access tests (Drupal\field\Tests\FieldAccessTest)
 - Field attach tests (other) (Drupal\field\Tests\FieldAttachOtherTest)
 - Field attach tests (storage-related) (Drupal\field\Tests\FieldAttachStorageTest)
 - Field info tests (Drupal\field\Tests\FieldInfoTest)
 - Field instance CRUD tests (Drupal\field\Tests\FieldInstanceCrudTest)
 - Field form tests (Drupal\field\Tests\FormTest)
 - Field translations tests (Drupal\field\Tests\TranslationTest)
 - Field translations web tests (Drupal\field\Tests\TranslationWebTest)
 - Field SQL Storage tests (Drupal\field_sql_storage\Tests\FieldSqlStorageTest)
 - Multilingual fields (Drupal\node\Tests\NodeFieldMultilingualTestCase)

Test run started:
 Sunday, March 3, 2013 - 21:58

Test summary
------------

Field active test 14 passes, 0 fails, and 0 exceptions
Field bulk delete tests 93 passes, 0 fails, and 0 exceptions
Field CRUD tests 121 passes, 0 fails, and 0 exceptions
Field Display API tests 58 passes, 0 fails, and 0 exceptions
Field access tests 21 passes, 0 fails, 0 exceptions, and 5 debug messages
Field attach tests (other) 139 passes, 0 fails, and 0 exceptions
Field attach tests (storage-related) 148 passes, 0 fails, and 0 exceptions
Field info tests 115 passes, 0 fails, and 0 exceptions
Field instance CRUD tests 52 passes, 0 fails, and 0 exceptions
Field form tests 281 passes, 0 fails, 0 exceptions, and 63 debug messages
Field translations tests 90 passes, 0 fails, and 2 exceptions
Field translations web tests 23 passes, 0 fails, 0 exceptions, and 4 debug messages
Field SQL Storage tests 132 passes, 0 fails, and 0 exceptions
Multilingual fields 64 passes, 0 fails, 0 exceptions, and 21 debug messages

Test run duration: 1 min 6 sec

I just love DUBT :)

The only reason it's still > 1min is because of the remaining web tests (Field active test, Field form tests (obviously), Field translation web tests and Multilingual fields, that last one is scheduled for removal in the Node NG issue I think.

Keep in mind that I've run those tests sequentally, it won't make such a big difference on the bot.

Status: Needs review » Needs work

The last submitted patch, field-dubt-1932382-4.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
field-dubt-1932382-7.patch42.24 KB

Fixed the translation tests, they require the node module to be active (and already had those exceptions in the output above, d'oh) and removed the dependency on EntityUnitTestBase.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs review » Needs work

Funny, while going back from London to Belgium this evening, I hacked further on this and did almost the exact same things :)
I've tried ripping apart the formTest merging all methods into one single test method but still getting failures locally, but maybe that's not really interesting for now.

You left a debug statement and that's actually what I had todo as well, this is the missing node entity right ? ;)

+++ b/core/modules/field/field.crud.incundefined
@@ -91,6 +91,9 @@ function field_create_field($field) {
+    if (!isset($info['entity_keys'])) {
+      debug($info);
+    }
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
field-dubt-1932382-9-interdiff.txt699 bytes
field-dubt-1932382-9.patch41.56 KB

I hoped you would be working on something else but I guess it's good that we ended up with almost the same code, even including the debug snippets :)

Re-rolled without that.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs review » Needs work

Two small things.

+++ b/core/modules/field/lib/Drupal/field/Tests/FieldUnitTestBase.phpundefined
@@ -0,0 +1,229 @@
+ * Definition of Drupal\field\Tests\FieldTestBase.

Should be FieldUnitTestBase

+++ b/core/modules/field/lib/Drupal/field/Tests/FieldUnitTestBase.phpundefined
@@ -0,0 +1,229 @@
+    //$this->installSchema('user', 'users');

I guess we can drop this then.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
field-dubt-1932382-11-interdiff.txt813 bytes
field-dubt-1932382-11.patch41.51 KB

Oh, forgot to remove that, yes. It's gone now.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs review » Reviewed & tested by the community

Alright, let's do this

yched’s picture

yched CreditAttribution: yched commented

Awesome !
While working on the CMI patch and battling with tests I was just thinking of the amount of tests that should be moved to DUTB...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Yay, faster tests! This should be nice for #SprintWeekend, within which testbot was backed up for 7 hours or so at one point. :P

Committed and pushed to 8.x. Thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +SprintWeekend2013

x

amateescu’s picture

amateescu CreditAttribution: amateescu commented
FileSize
1932382-followup.patch949 bytes

Unbreaking HEAD :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Oops. :P

Committed and pushed to 8.x. Thanks!

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

I've added a note about this new base class to #1902098: Introduce DUTB helper methods for working with rendered content + FieldPluginUnitTestBase

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Updated issue summary.