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

Unit Tests: switch to PHP 7.4 Production #14171

Closed
wants to merge 10 commits into from
Closed

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Dec 4, 2019

Changes proposed in this Pull Request:

  • Now that PHP 7.4 has shipped, we should be able to use the final stable version in Unit Tests.

Testing instructions:

  • Not much to test here, just see if the tests pass.

Proposed changelog entry for your changes:

  • N/A

@jeherve jeherve added [Status] Needs Review To request a review from Crew. Label will be renamed soon. Unit Tests [Type] Janitorial [Pri] Low labels Dec 4, 2019
@jeherve jeherve added this to the 8.1 milestone Dec 4, 2019
@jeherve jeherve requested a review from kraftbj December 4, 2019 08:01
@jeherve jeherve requested a review from a team as a code owner December 4, 2019 08:01
@jeherve jeherve self-assigned this Dec 4, 2019
@jetpackbot
Copy link

jetpackbot commented Dec 4, 2019

Warnings
⚠️ The Privacy section is missing for this PR. Please specify whether this PR includes any changes to data or privacy.
⚠️

pre-commit hook was skipped for one or more commits

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-14171

This is an 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 386fcf9

@kraftbj
Copy link
Contributor

kraftbj commented Dec 4, 2019

Couple of tests are failing in 7.4, but sounds like issues with the tests themselves.

There were 2 errors:

1) WP_Test_Jetpack_Photon::test_photon_cdn_in_rest_response_with_view_context

PHPUnit_Framework_Exception: Argument #2 (No Value) of PHPUnit_Framework_Assert::assertArrayHasKey() must be a array or ArrayAccess

/tmp/wordpress-master/src/wp-content/plugins/jetpack/tests/php/test_class.jetpack_photon.php:1040

2) WP_Test_Jetpack_Photon::test_photon_cdn_in_rest_response_with_edit_context

PHPUnit_Framework_Exception: Argument #2 (No Value) of PHPUnit_Framework_Assert::assertArrayHasKey() must be a array or ArrayAccess

/tmp/wordpress-master/src/wp-content/plugins/jetpack/tests/php/test_class.jetpack_photon.php:1065

@jeherve jeherve force-pushed the update/travis-74 branch 2 times, most recently from 7e21a75 to 0bc7f3d Compare December 5, 2019 17:06
@kraftbj
Copy link
Contributor

kraftbj commented Dec 5, 2019

I updated #13773 to give us a playground to work on this test.

@kraftbj kraftbj self-assigned this Dec 5, 2019
@jeherve jeherve added [Status] Blocked / Hold and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Dec 6, 2019
@jeherve
Copy link
Member Author

jeherve commented Dec 6, 2019

It turns out PHP 7.4 is not yet ready for Ubuntu Trusty (14.04) on Travis. Work is in progress here:
php-build/php-build#599

Putting this PR on hold for now.

@kraftbj
Copy link
Contributor

kraftbj commented Dec 6, 2019

Alternatively, we can explore not using Trusty. An initial test led to a lot more failures, so we can examine that on it's own and not within the context of this PR if php-build's work beats us to it.

@jeherve jeherve removed this from the 8.1 milestone Dec 20, 2019
jeherve added a commit that referenced this pull request Mar 6, 2020
We're still running into the same issues as in #14171
zinigor pushed a commit that referenced this pull request Mar 9, 2020
* General: update minimum WordPress version for Jetpack

WordPress 5.4 is scheduled for release before the next version of Jetpack will ship.

Let's consequently aim to update Jetpack to keep our WP-1 support standard.

* Remove temporary workarounds now that all Jetpack users use WP5.3+

Also update date() into gmdate() as per WordPress coding standards changes.

* Related Posts: remove workaround that is not necessary anymore

* Revert Travis changes for now

We're still running into the same issues as in #14171

* Force all "Previous" WP version tests to be 5.3 for now

We can move back to "previous" once WP 5.4 has shipped
@stale
Copy link

stale bot commented Mar 30, 2020

This PR has been marked as stale. This happened because:

  • It has been inactive in the past 3 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this PR has clear testing instructions, is it up to date with master, and it is still valid. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@stale stale bot added the [Status] Stale label Mar 30, 2020
@stale stale bot removed the [Status] Stale label Apr 9, 2020
@kraftbj kraftbj removed the request for review from a team May 26, 2020 22:29
@kraftbj
Copy link
Contributor

kraftbj commented Jul 8, 2020

Still fiddling with it. When I dump the value, it is there, but obviously is sometimes a class instead of an array. I added some json encoding/decoding to standardize everything as an array, but still instances of Core not returning what we'd expect.

This isn't an unit test failure as much as an integration test failure, but still not sure what's happening yet.

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.

None yet

3 participants