Expand Drupal6FieldInstance testing with a float field.

We found that some tests were failing because the test mixed decimals and float field types. We actually switched the test to decimal for getting green, but it will be awesome to add coverage for float fields migration.

CommentFileSizeAuthor
#7 interdiff.txt715 bytesbenjy
#7 2226395-7.patch4.62 KBbenjy
#5 2226395-5.patch4.7 KBbenjy

Comments

David Hernández’s picture

David Hernández commented
Assigned: Unassigned » David Hernández

I'm working on this.

David Hernández’s picture

David Hernández commented
Assigned: David Hernández » Unassigned
Status: Active » Needs work

I can't add this as the tests to do it are failing:

Fatal error: Call to a member function getLabel() on a non-object in core/modules/migrate_drupal/lib/Drupal/migrate_drupal/Tests/d6/MigrateFieldInstanceTest.php on line 79

    $field = entity_load('field_instance_config', 'node.story.field_test');
    $this->assertEqual($field->getLabel(), 'Text Field');

The entity is coming in as NULL and I have no idea of what's going on. I can continue working on this, but I would need that someone points me in the right direction.

benjy’s picture

benjy commented
Status: Needs work » Active

I've fixed MigrateFieldInstanceTest in a2587fb so this should be good to work on now.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented
Project: IMP » Drupal core
Version: » 8.x-dev
Component: Code » migration system
Issue tags: +Needs tests
benjy’s picture

benjy commented
Status: Active » Needs review
StatusFileSize
new 2226395-5.patch4.7 KB

This was a little strange, the dumps were mixed up. We had a decimal field but a float field instance. I fixed that up and added a float so we have one of each. I also added another test in the MigrateFieldInstance test but that wouldn't really have caught the issue since we were hand creating the field in the setup with the right type.

This issue would however would be caught by the Drupal6Migrate test which was added after this was fixed.

Status: Needs review » Needs work

The last submitted patch, 5: 2226395-5.patch, failed testing.

benjy’s picture

benjy commented
Status: Needs work » Needs review
StatusFileSize
new 2226395-7.patch4.62 KB
new interdiff.txt715 bytes

Messed up the dump.

chx’s picture

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

> We had a decimal field but a float field instance.

NICE. Thanks for catching that.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue tags: -Needs tests
webchick’s picture

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

Committed and pushed to 8.x. Thanks!

  • Commit b452ac5 on 8.x by webchick: 
    Issue #2226395 by benjy | penyaskito: Expand Drupal6FieldInstance...

Status: Fixed » Closed (fixed)

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