Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 7.1.0RC4 test failures on master #26488

Closed
PVince81 opened this issue Oct 26, 2016 · 8 comments · Fixed by #26502
Closed

Fix 7.1.0RC4 test failures on master #26488

PVince81 opened this issue Oct 26, 2016 · 8 comments · Fixed by #26502

Comments

@PVince81
Copy link
Contributor

00:23:26 There were 4 failures:
00:23:26 
00:23:26 1) Test\Comments\ManagerTest::testGetComment
00:23:26 Failed asserting that two DateTime objects are equal.
00:23:26 --- Expected
00:23:26 +++ Actual
00:23:26 @@ @@
00:23:26  2016-10-25T22:22:04+0000
00:23:26 
00:23:26 /var/lib/jenkins/workspace/owncloud-core_core_PR-26476-MNN7CDVM5JONKOMQNADXUNNP62OTCAGAR5OFYESKH4STOA3OS3EQ/tests/lib/Comments/ManagerTest.php:110
00:23:26 
00:23:26 2) Test\Comments\ManagerTest::testSaveNew
00:23:26 Failed asserting that two DateTime objects are equal.
00:23:26 --- Expected
00:23:26 +++ Actual
00:23:26 @@ @@
00:23:26  2016-10-25T22:22:04+0000
00:23:26 
00:23:26 /var/lib/jenkins/workspace/owncloud-core_core_PR-26476-MNN7CDVM5JONKOMQNADXUNNP62OTCAGAR5OFYESKH4STOA3OS3EQ/tests/lib/Comments/ManagerTest.php:372
00:23:26 
00:23:26 3) Test\Comments\ManagerTest::testSetMarkRead
00:23:26 Failed asserting that two DateTime objects are equal.
00:23:26 --- Expected
00:23:26 +++ Actual
00:23:26 @@ @@
00:23:26  2016-10-25T22:22:04+0000
00:23:26 
00:23:26 /var/lib/jenkins/workspace/owncloud-core_core_PR-26476-MNN7CDVM5JONKOMQNADXUNNP62OTCAGAR5OFYESKH4STOA3OS3EQ/tests/lib/Comments/ManagerTest.php:576
00:23:26 
00:23:26 4) Test\Share20\DefaultShareProviderTest::testCreateLinkShare
00:23:26 Failed asserting that two DateTime objects are equal.
00:23:26 --- Expected
00:23:26 +++ Actual
00:23:26 @@ @@
00:23:26  2016-10-25T22:22:22+0000
00:23:26 
00:23:26 /var/lib/jenkins/workspace/owncloud-core_core_PR-26476-MNN7CDVM5JONKOMQNADXUNNP62OTCAGAR5OFYESKH4STOA3OS3EQ/tests/lib/Share20/DefaultShareProviderTest.php:747
00:23:26 
00:23:26 --

@DeepDiver1975 @PhilippSchaffrath

@PVince81
Copy link
Contributor Author

PVince81 commented Oct 27, 2016

There is something fishy in the comments code. The database column is called creation_timestamp, however the Comment object is reading creationDT. I'm still trying to find where these values are mapped... If not mapped, it is likely that the test didn't fail before because new DateTime() with a null object would create a date with the current time. And with this PHP version it seems new DateTime(null) creates an empty datetime instead.

Let's find out...

@PVince81
Copy link
Contributor Author

PHP 5.6:

creationDT: {"date":"2016-10-27 15:06:28.000000","timezone_type":3,"timezone":"UTC"}
object date {"date":"2016-10-27 15:06:28.000000","timezone_type":3,"timezone":"UTC"}

PHP 7.1.0RC4:

creationDT: {"date":"2016-10-27 15:05:26.581691","timezone_type":3,"timezone":"UTC"}
getCreationDateTime {"date":"2016-10-27 15:05:26.000000","timezone_type":3,"timezone":"UTC"}

When saving into the DB the milliseconds part is lost.
Looks like PHP 7.1 does generate the milliseconds part in the original date.

So I guess it's just a matter of adjusting the test to ignore the ms part when checking.

@PVince81
Copy link
Contributor Author

And 7.1.0RC1 also didn't generate the milliseconds part, but RC4 does it.
This is when creating a plain 'DateTime` object:

± % phpenv local 7.1.0RC1
phpenv v0.0.4-dev

7.1.0RC1

vincent@petry ~/work/workspace/owncloud [master *]
± % php ~/temp/test.php
object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2016-10-27 15:08:29.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "GMT"
}
object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2016-10-27 15:08:29.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "GMT"
}

vincent@petry ~/work/workspace/owncloud [master *]
± % phpenv local 7.1.0RC4
phpenv v0.0.4-dev

7.1.0RC4

vincent@petry ~/work/workspace/owncloud [master *]
± % php ~/temp/test.php
object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2016-10-27 15:08:32.537339"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "GMT"
}
object(DateTime)#1 (3) {
  ["date"]=>
  string(26) "2016-10-27 15:08:32.537469"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "GMT"

@PVince81
Copy link
Contributor Author

Ok, looks like it's intentional: https://bugs.php.net/bug.php?id=73247

@PVince81
Copy link
Contributor Author

Fix is here: #26502

@phisch
Copy link
Contributor

phisch commented Oct 27, 2016

Well, it's not really intentional, its a bug existing in RC 1-3. Working on a fix on our side here does not seem right.

Personally i would just update jenkins to rc4 and ignore those tests failing in rc1-3. What do you think about that?

@PVince81
Copy link
Contributor Author

Jenkins is already updated to RC4.

These tests passed on RC1-3. The "problem" is introduced in RC4 by PHP when they fixed the DateTime behavior.

I think the way the PR is written is the correct approach. Even if older PHP versions would provide a microtime, that one doesn't get written to the DB so the assert comparison was wrong anyway. So fixing that is the way to go.

@lock
Copy link

lock bot commented Jul 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants