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

Add test and fix for date_constructor for PHP Versions lower 7.1.0 #15853

Merged
merged 2 commits into from May 10, 2017

Conversation

@rdeutz
Copy link
Contributor

rdeutz commented May 6, 2017

Summary of Changes

Fix for JDate Constructor, integrates the test case from #15642 but with true as 2nd parameter for the format method (default is false and means that we get the time in GMT)

Testing Instructions

Test should be processed with two different PHP Versions one lower 7.1.0 and one 7.1.0 or greater. The results should be the "same" for all versions after the patch is applied.

  • Add the follwing PHP code to your template:
echo 'Timezone US/Central: ';
$jdateUS   = new JDate('now', new DateTimeZone('US/Central'));
$phpdateUS = new DateTime('now', new DateTimeZone('US/Central'));
echo $jdateUS->format('D m/d/Y H:i', true) . ' | ' . $phpdateUS->format('D m/d/Y H:i');
echo '<br>';
echo 'Timezone UTC: ';
$jdateUTC   = new JDate('now', new DateTimeZone('UTC'));
$phpdateUTC = new DateTime('now', new DateTimeZone('UTC'));
echo $jdateUTC->format('D m/d/Y H:i', true) . ' | ' . $phpdateUTC->format('D m/d/Y H:i');
echo '<br>';
echo 'Timezone not set: ';
$jdateNoTZ   = new JDate('now');
$phpdateNoTZ = new DateTime('now');
echo $jdateNoTZ->format('D m/d/Y H:i', true) . ' | ' . $phpdateNoTZ->format('D m/d/Y H:i');
  • Access your site

  • Change the PHP version

  • Access your site again

  • Apply patch

  • Access your site

  • Change the PHP version

  • Access your site again

Test is successful when after appling the patch you get correct results, time in a row must be the same expect the last line.

Results

Keep in mind your results are depending on your timezone and the time you are running the test. For me the 2 hour difference in the "Timezone not set" row is ok because I am on CEST what is GMT+2.

The Error is in the "Timezone US/Central" row for PHP Version lower 7.1.0, in my case 5.6.30.

Before Patch:

7.1.1 (local time 14:29)
Timezone US/Central: Sat 05/06/2017 07:29 | Sat 05/06/2017 07:29
Timezone UTC: Sat 05/06/2017 12:29 | Sat 05/06/2017 12:29
Timezone not set: Sat 05/06/2017 12:29 | Sat 05/06/2017 14:29

5.6.30 (local time 14:31)
Timezone US/Central: Sat 05/06/2017 12:31 | Sat 05/06/2017 07:31
Timezone UTC: Sat 05/06/2017 12:31 | Sat 05/06/2017 12:31
Timezone not set: Sat 05/06/2017 12:31 | Sat 05/06/2017 14:31

After Patch

7.1.1 (local time 14:35)
Timezone US/Central: Sat 05/06/2017 07:35 | Sat 05/06/2017 07:35
Timezone UTC: Sat 05/06/2017 12:35 | Sat 05/06/2017 12:35
Timezone not set: Sat 05/06/2017 12:35 | Sat 05/06/2017 14:35

5.6.30 (local time 14:34)
Timezone US/Central: Sat 05/06/2017 07:34 | Sat 05/06/2017 07:34
Timezone UTC: Sat 05/06/2017 12:34 | Sat 05/06/2017 12:34
Timezone not set: Sat 05/06/2017 12:34 | Sat 05/06/2017 14:34

Documentation Changes Required

None

@rdeutz rdeutz requested a review from mbabker May 6, 2017
@rdeutz rdeutz added this to the Joomla 3.7.1 milestone May 6, 2017
{
$date = parent::createFromFormat('U.u', number_format(microtime(true), 6, '.', ''), $tz)->format('Y-m-d H:i:s.u');
$data = time();

This comment has been minimized.

Copy link
@dgrammatiko

dgrammatiko May 6, 2017

Contributor

$data or $date ?

This comment has been minimized.

Copy link
@rdeutz

rdeutz May 6, 2017

Author Contributor

shit

If the PHP documentation is right then time() is not a valid value to create a datetime object
@rdeutz
Copy link
Contributor Author

rdeutz commented May 6, 2017

I did remove the version check, seems to me not really important if JDate has microseconds or not

@mbabker
mbabker approved these changes May 6, 2017
@laoneo
Copy link
Member

laoneo commented May 10, 2017

I have tested this item successfully on ccbe09d

Here are my test results:

Test with patch and PHP 7.1.3:
Timezone US/Central: Wed 05/10/2017 02:00 | Wed 05/10/2017 02:00
Timezone UTC: Wed 05/10/2017 07:00 | Wed 05/10/2017 07:00
Timezone not set: Wed 05/10/2017 07:00 | Wed 05/10/2017 07:00

Test with patch and PHP 5.6.20:
Timezone US/Central: Wed 05/10/2017 02:03 | Wed 05/10/2017 02:03
Timezone UTC: Wed 05/10/2017 07:03 | Wed 05/10/2017 07:03
Timezone not set: Wed 05/10/2017 07:03 | Wed 05/10/2017 07:03


Test without patch and PHP 7.1.3:
Timezone US/Central: Wed 05/10/2017 02:05 | Wed 05/10/2017 02:05
Timezone UTC: Wed 05/10/2017 07:05 | Wed 05/10/2017 07:05
Timezone not set: Wed 05/10/2017 07:05 | Wed 05/10/2017 07:05

Test without patch and PHP 5.6.20:
Timezone US/Central: Wed 05/10/2017 07:04 | Wed 05/10/2017 02:04
Timezone UTC: Wed 05/10/2017 07:04 | Wed 05/10/2017 07:04
Timezone not set: Wed 05/10/2017 07:04 | Wed 05/10/2017 07:04

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15853.
@gwsdesk
Copy link

gwsdesk commented May 10, 2017

I have tested this item successfully on ccbe09d See http://gwsdev1.xyz

Note all versions of PHP with PHP-FPM = On and also tested with PHP-FPM = Off

Tests:

PHP 7.1.14 without patch

Timezone US/Central: Wed 05/10/2017 09:02 | Wed 05/10/2017 09:02
Timezone UTC: Wed 05/10/2017 14:02 | Wed 05/10/2017 14:02
Timezone not set: Wed 05/10/2017 14:02 | Wed 05/10/2017 09:02

PHP 7.1.14 with patch

Timezone US/Central: Wed 05/10/2017 09:07 | Wed 05/10/2017 09:07
Timezone UTC: Wed 05/10/2017 14:07 | Wed 05/10/2017 14:07
Timezone not set: Wed 05/10/2017 14:07 | Wed 05/10/2017 09:07

PHP 5.6.30 without patch

Timezone US/Central: Wed 05/10/2017 14:08 | Wed 05/10/2017 09:08
Timezone UTC: Wed 05/10/2017 14:08 | Wed 05/10/2017 14:08
Timezone not set: Wed 05/10/2017 14:08 | Wed 05/10/2017 09:08

PHP 5.6.30 with patch

Timezone US/Central: Wed 05/10/2017 09:09 | Wed 05/10/2017 09:09
Timezone UTC: Wed 05/10/2017 14:09 | Wed 05/10/2017 14:09
Timezone not set: Wed 05/10/2017 14:09 | Wed 05/10/2017 09:09

PHP 7.0 without patch

Timezone US/Central: Wed 05/10/2017 14:10 | Wed 05/10/2017 09:10
Timezone UTC: Wed 05/10/2017 14:10 | Wed 05/10/2017 14:10
Timezone not set: Wed 05/10/2017 14:10 | Wed 05/10/2017 09:10

PHP 7.0 with patch
Timezone US/Central: Wed 05/10/2017 09:11 | Wed 05/10/2017 09:11
Timezone UTC: Wed 05/10/2017 14:11 | Wed 05/10/2017 14:11
Timezone not set: Wed 05/10/2017 14:11 | Wed 05/10/2017 09:11

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.1 milestone May 10, 2017
@ghost
Copy link

ghost commented May 10, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label May 10, 2017
@jeckodevelopment jeckodevelopment merged commit 5e916c0 into joomla:staging May 10, 2017
4 checks passed
4 checks passed
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@joomla-cms-bot joomla-cms-bot removed the RTC label May 10, 2017
@jeckodevelopment jeckodevelopment added this to the Joomla 3.7.1 milestone May 10, 2017
@rdeutz rdeutz deleted the rdeutz:fix_date_contructor branch May 11, 2017
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request May 16, 2017
Add test and fix for date_constructor for PHP Versions lower 7.1.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.