Skip to content

Conversation

@simison
Copy link
Member

@simison simison commented Nov 27, 2018

Add 7.3

Adds PHP v7.3 containers for CI testing. PHP v7.3 is scheduled for December 6th.

Note that 7.3 is aliassed to RC versions in Travis and it's currently running RC5:

$ php --version
PHP 7.3.0RC5 (cli) (built: Nov 12 2018 00:13:27) ( ZTS )

Dropping 7.0?

With this, should we drop testing in 7.0 as security updates for it will end on Dec 5th 2018 (in a few days)?
http://php.net/supported-versions.php

image

@simison simison requested a review from a team November 27, 2018 11:13
@jetpackbot
Copy link
Collaborator

jetpackbot commented Nov 27, 2018

Warnings
⚠️ "Testing instructions" are missing for this PR. Please add some
⚠️ "Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 02ba8cc

@simison
Copy link
Member Author

simison commented Nov 27, 2018

This brings up two failed tests:

There were 2 failures:

1) WP_Test_Jetpack_PHP_Lint::test_php_lint
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-site-settings-endpoint.php on line 823
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-update-post-endpoint.php on line 762
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-update-post-endpoint.php on line 771
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-update-post-v1-1-endpoint.php on line 841
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-update-post-v1-1-endpoint.php on line 850
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-update-post-v1-2-endpoint.php on line 812
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./json-endpoints/class.wpcom-json-api-update-post-v1-2-endpoint.php on line 821
Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in ./modules/custom-css/csstidy/class.csstidy.php on line 858
Failed asserting that an array is empty.
/tmp/wordpress-master/src/wp-content/plugins/jetpack/tests/php/test_php-lint.php:24

2) WP_Test_Jetpack_Sync_SSO::test_sync_sso_is_two_step_required_filter_true
Failed asserting that null is true.
/tmp/wordpress-master/src/wp-content/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-sso.php:16

@jeherve jeherve force-pushed the update/travis-php730 branch from 93713ba to 18c14a3 Compare January 2, 2019 09:06
@jeherve
Copy link
Member

jeherve commented Jan 2, 2019

I rebased after merging #11030, but some tests still fail, all because of issues with this:

PHP Notice:  compact(): Undefined variable: title in /tmp/wordpress-latest/src/wp-content/plugins/jetpack/tests/php/sync/test_class.jetpack-sync-plugins.php on line 198

https://secure.php.net/manual/en/function.compact.php#refsect1-function.compact-changelog

compact() now issues notices when a variable is not set.
secure.php.net/manual/en/function.compact.php#refsect1-function.compact-changelog
@jeherve jeherve force-pushed the update/travis-php730 branch from 998b203 to fff2545 Compare January 2, 2019 14:28
@jeherve jeherve added [Status] Needs Review This PR is ready for review. [Package] Sync and removed [Status] In Progress labels Jan 2, 2019
@jeherve jeherve added this to the 6.9 milestone Jan 2, 2019
@jeherve jeherve added [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Jan 2, 2019
@jeherve jeherve modified the milestones: 6.9, 7.0 Jan 2, 2019
WP 5.0 was made compatible with PHP 7.3 in this changeset:
https://core.trac.wordpress.org/changeset/44166
https://core.trac.wordpress.org/ticket/44416

Unit tests for WP Versions that come before that will always fail, because of the following errors:
compact(): Undefined variable: context
https://core.trac.wordpress.org/changeset/44166#file6

Once WP_BRANCH=previous becomes 5.0.0, we can update Travis to run PHP 7.3 tests for all our Matrix.
@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Jan 22, 2019
@jeherve
Copy link
Member

jeherve commented Jan 22, 2019

I've made some changes and this should now be good for a review. The tests now pass.

02ba8cc is worth mentioning:

we will not run PHP 7.3 tests on WP 4.9.9.

WP 5.0 was made compatible with PHP 7.3 in this changeset:
https://core.trac.wordpress.org/changeset/44166
https://core.trac.wordpress.org/ticket/44416

Unit tests for WP Versions that come before that will always fail, because of the following errors:
compact(): Undefined variable: context
This was addressed in Core in WP 5.0 here:
https://core.trac.wordpress.org/changeset/44166#file6

Once WP_BRANCH=previous becomes 5.0, we can update Travis to run PHP 7.3 tests for all our Matrix.

@jeherve jeherve modified the milestones: 7.0, 7.1 Jan 28, 2019
Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it and it tests as expected.

@kraftbj kraftbj added the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 13, 2019
@kraftbj kraftbj removed the [Status] Needs Review This PR is ready for review. label Feb 13, 2019
@jeherve jeherve merged commit 001089a into master Feb 15, 2019
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 15, 2019
@jeherve jeherve deleted the update/travis-php730 branch February 15, 2019 11:22
jeherve added a commit that referenced this pull request Feb 22, 2019
#### Changes proposed in this Pull Request:

Follow up from #10723. 

We previously had to limit PHP 7.3 to WordPress versions that supported it.
Now that WordPress 5.1 was released, even the previous version of WP (5.0) supports PHP 7.3.
We can consequently run tests globally.

Master issue: #10729 

#### Testing instructions:

* Do the tests pass?
* Smile and approve.

#### Proposed changelog entry for your changes:

* None
jeherve added a commit that referenced this pull request Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants