Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Current tests only account for 1:1 relationship between fields and instances.
We should test the case of several instances for a field.
Comment | File | Size | Author |
---|---|---|---|
#19 | more_tests_for_field_config-1960574-19.patch | 15.81 KB | swentel |
#16 | more_tests_for_field_config-1960574-16.patch | 16.88 KB | swentel |
#13 | more_tests_for_field_config-1960574-13.patch | 16.86 KB | smiletrl |
#13 | interdiff-10-13.txt | 8.79 KB | smiletrl |
#11 | more_tests_for_field_config-1960574-10.patch | 16.84 KB | smiletrl |
Comments
Comment #1
yched CreditAttribution: yched commentedPostponed on #1735118: Convert Field API to CMI
Comment #2
swentel CreditAttribution: swentel commentedtagging
Comment #3
yched CreditAttribution: yched commentedUn-postponing.
Comment #4
swentel CreditAttribution: swentel commentedActually, we test it for the body instance:
So I think we're good imo.
Comment #5
yched CreditAttribution: yched commentedThat's just for the upgrade path.
I think @xjm was thinking of the config import tests.
Comment #6
swentel CreditAttribution: swentel commentedright, duh, I need to learn to read titles.
Comment #7
swentel CreditAttribution: swentel commentedComment #9
swentel CreditAttribution: swentel commentedRight, more fields, means more stuff to delete :)
Comment #10
smiletrl CreditAttribution: smiletrl commentedNice job!
Some nitpicks though:^
FieldImportCreateTest.php:
I think here should be
This patch doesn't check $field_2 and $instance_2 before enabling field_test_config module.
It might be a good thing to add this multiple instances test in
// Add the coresponding entries to the current manifest data.
This is not from the patch, but in the original code -- wrong word *corresponding*.
FieldImportDeleteTest.php:
I think you want $instance_id_2b to has a different bundle --
+ $instance_id_2b = "test_entity.test_bundle_2.$field_id_2";
and $instance_id_2a has a different field id -- $field_id_2:)Any particular reason to delete these checks? In this test file, it uses
$this->installConfig(array('field_test_config'));
to enable configuration, different fromin FieldImportCreateTest.php. I think it's necessary to add this check and for new field and instances in this patch.
Could be
$field_uuid_2 = entity_load('field_entity', $field_id_2)->uuid();
, although I prefer this way$field->uuid
^^Finally, while this patch creates a new field and two based different instances, I guess a second instance with a different bundle based on current field in these tests will be okay:)
Comment #11
smiletrl CreditAttribution: smiletrl commentedPatch updated according to #10.
For 'uuid' in staging yml file, I used following code to generate them. Not sure I did it correctly.
Comment #13
smiletrl CreditAttribution: smiletrl commentedFixed some bugs.
Comment #14
swentel CreditAttribution: swentel commentedNeeds a reroll
Comment #15
YesCT CreditAttribution: YesCT commentedtagging.
also, instructions if someone new wants to try it: http://drupal.org/patch/reroll
Comment #16
swentel CreditAttribution: swentel commentedComment #17
swentel CreditAttribution: swentel commentedThis is good to go btw, mine was just a re-roll.
Comment #18
swentel CreditAttribution: swentel commentedManifests are gone as of #1998576: Make the config import process use full config trees again, so this needs work
Comment #19
swentel CreditAttribution: swentel commentedAnd here's the reroll.
Comment #20
alexpottCommitted 939a8a8 and pushed to 8.x. Thanks!