The test \Drupal\Tests\feeds\Unit\Feeds\Target\DateTimeTest fails when the test is ran by the testbot. Strangely, I got no errors when running phpunit tests from the commandline this way:

cd modules/wip/feeds/feeds8
phpunit --group feeds

But there is a failure when running the test this way from the command line:

php core/scripts/run-tests.sh --file modules/wip/feeds/feeds8/tests/src/Unit/Feeds/Target/DateTimeTest.php

The fail happens on the following line:

$this->assertSame($values['value'], '2000-01-01T00:00:00');

When logging that $values['value'] value, it appears to be "1999-12-31T13:00:00" instead, so it looks the issue has something to do with timezones.

In \Drupal\feeds\Feeds\Target\DateTime::prepareValue() the year value is converted to "January 2000" which is then passed to strtotime(). I think it makes sense to specify the timezone in this string (which should be "UTC" as that is what the value of the constant DATETIME_STORAGE_TIMEZONE is).

Patch will follow.

CommentFileSizeAuthor
#4 2595673.4.patch1.79 KBalexpott
#2 feeds8-datetime-test-2595673-2.patch463 bytesMegaChriz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MegaChriz created an issue. See original summary.

MegaChriz’s picture

MegaChriz CreditAttribution: MegaChriz as a volunteer commented
Assigned: MegaChriz » Unassigned
Status: Active » Needs review
FileSize
feeds8-datetime-test-2595673-2.patch463 bytes

Let's see if this patch passes all tests.

MegaChriz’s picture

MegaChriz CreditAttribution: MegaChriz as a volunteer commented
Status: Needs review » Needs work

While the patch in #2 fixes the test failure, I don't think it is the right fix. I think the "fix" hides issues related to time zones. I hope to be more knowledgable about fixing this properly after I have dived into #722740: Feeds Date mapper converts imported dates to GMT unless in UNIX timestamp format.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs work » Needs review
FileSize
2595673.4.patch1.79 KB

It think one issue is that strtotime will use the timezone that is set. The reason the test fails is that we've intentionally done...

// Set the default timezone. While this doesn't cause any tests to fail, PHP
// complains if 'date.timezone' is not set in php.ini. The Australia/Sydney
// timezone is chosen so all tests are run using an edge case scenario (UTC+10
// and DST). This choice is made to prevent timezone related regressions and
// reduce the fragility of the testing system in general.
date_default_timezone_set('Australia/Sydney');

In the unit test bootstrap.

  • twistor committed 719613e on 8.x-3.x authored by alexpott 
    Issue #2595673 by MegaChriz, alexpott: Test Drupal\Tests\feeds\Unit\...
twistor’s picture

twistor CreditAttribution: twistor as a volunteer commented
Status: Needs review » Fixed

Going with this for now. Needs to be revisited with regard to #722740: Feeds Date mapper converts imported dates to GMT unless in UNIX timestamp format.

Status: Fixed » Closed (fixed)

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