https://www.drupal.org/pift-ci-job/31459:

fail: [Other] Line 130 of core/modules/node/src/Tests/Migrate/d7/MigrateNodeTest.php:
Value '1' is identical to value '1.0'.

New since yesterday. Was introduced in this commit:

commit 10ea52eaa23284ef213523483435ffe65eef12cd
Author: webchick <drupal@webchick.net>
Date:   Wed Sep 9 15:09:01 2015 -0700

    Issue #2563819 by phenaproxima, mikeryan: [D7] Field display settings do not migrate

So if this isn't a really quick fix, we can always roll back #2563819: [D7] Field display settings do not migrate (though that is a "Migrate critical" so it would be nice to avoid it.)

Here's the test:

  /**
   * Test node migration from Drupal 7 to 8.
   */
  public function testNode() {
    $this->assertEntity(1, 'test_content_type', 'en', 'A Node', '2', TRUE, '1421727515', '1441032132', TRUE, FALSE);
    $this->assertRevision(1, 'A Node', '2', NULL, '1441032132');

    $node = Node::load(1);
    $this->assertTrue($node->field_boolean->value);
    $this->assertIdentical('99-99-99-99', $node->field_phone->value);
    $this->assertIdentical('1', $node->field_float->value); # Line 130.
    $this->assertIdentical('5', $node->field_integer->value);
    $this->assertIdentical('Some more text', $node->field_text_list[0]->value);
    $this->assertIdentical('7', $node->field_integer_list[0]->value);
    $this->assertIdentical('qwerty', $node->field_text->value);
  }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick created an issue. See original summary.

chx’s picture

That issue is not Migrate critical but a Migrate D7 critical (this is why I insisted on splitting the tags!) which is not even a Drupal 8.1.0 target necessarily compared to this which is a Drupal 8.0.0 RC1 target. Let's roll it back and then figure it out over there.

amateescu’s picture

Status: Active » Needs review
FileSize
827 bytes

Yeah, SQLite is very sensitive with floats. This would be the easy fix.

webchick’s picture

Wonder if we should invert that and do assertIdentical on 1.0. Weird that this passes MySQL, even. You'd think a float would be a float.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I think, its exactly what we test. Its the fact that PgSql actually understands what a float is.

amateescu’s picture

MySQL (and most likely Postgres too) are trimming trailing zeros by default, so we would need to use FORMAT() on the float value at select-time if we want the output to be 1.0 on all database drivers.

quietone’s picture

Some time ago in Convert assetEqual to assertIdentical in migrate_drupal it was agreed to use assertIdentical in all cases for migration. If there is no other solution possible please add a comment to state why assertEqual is being used instead of assertIdentical in this instance.

dawehner’s picture

MySQL (and most likely Postgres too) are trimming trailing zeros by default, so we would need to use FORMAT() on the float value at select-time if we want the output to be 1.0 on all database drivers.

Well, in order to know we would need to know the schema of our database tables at runtime, which is a total different new wormhole.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +Regression

Yeah, let's just do the simplest thing that can possibly work here. Thanks a lot for the fast turnaround, amateescu!

Per quietone, commented that line with:

    // Use assertEqual() here instead, since SQLite interprets floats strictly.
    $this->assertEqual('1', $node->field_float->value);

Committed and pushed to 8.0.x. Thanks!

  • webchick committed bcf90b1 on 8.0.x
    Issue #2566723 by amateescu: New test fail on PHP 5.5 & SQLite 3.8
    
amateescu’s picture

If we really want to keep the assertIdentical() check, we could do it like this. Basically, as pointed out in http://stackoverflow.com/a/7466548/1499564, this is just a rendering issue.

    ...
    // MySQL and PostgreSQL are trimming trailing zeros on float data types so
    // we need to adjust the output of this value.
    $this->assertIdentical(sprintf('%.1f', '1'), $node->field_float->value);
    ...

Edit: cross-post with #9 :)

webchick’s picture

That seems a lot less readable. :) We'll see what the migrate team has to say when they wake up in a few hours.

mikeryan’s picture

Yes, the assertEqual is clearer, let's leave it be.

Thanks for getting that fixed quickly!

chx’s picture

I agree with #7

please add a comment to state why assertEqual is being used instead of assertIdentical in this instance.

webchick’s picture

chx’s picture

Thanks!

Status: Fixed » Closed (fixed)

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