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

PHP Unit Tests: Use global transients #37122

Merged

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Dec 4, 2021

Description

This PR is a just a test to see whether changing to the pre_site_transient_filter fixes the PHP unit tests, reflecting the changes in https://core.trac.wordpress.org/changeset/52311

How has this been tested?

Silently. Waiting in the dark until the PHP Unit tests pass.

Screenshots

Screen Shot 2021-12-04 at 2 57 03 pm

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

…HP unit tests, reflecting the change in wordpress/develop 54499
@ramonjd ramonjd changed the title WIP PHP Unit Tests: Use global transients Dec 4, 2021
@ramonjd ramonjd marked this pull request as ready for review December 4, 2021 04:05
@ramonjd ramonjd self-assigned this Dec 4, 2021
@ramonjd ramonjd added [Type] Bug An existing feature does not function as intended [Type] Question Questions about the design or development of the editor. labels Dec 4, 2021
@ramonjd
Copy link
Member Author

ramonjd commented Dec 4, 2021

From here:

Does that mean we'd have to update https://github.com/WordPress/gutenberg/blob/36746203fb97e715d709adbc9e9ff71037a42f14/lib/class-wp-rest-url-details-controller.php with the same changes as https://core.trac.wordpress.org/changeset/52311?

I have no context about why Gutenberg has it's own WP_REST_URL_Details_Controller class.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 5, 2021

Good catch, @ramonjd 👍

I have no context about why Gutenberg has it's own WP_REST_URL_Details_Controller class.

The plugin has to support older versions of WP, and this is the new API available in 5.9. The WP_REST_URL_Details_Controller class is conditionally loaded. This and unit tests being out of sync is also the reason for test failures.

Maybe we should start removing unit tests for APIs that are part of WP core since updates should be submitted to the core anyways.

@ockham
Copy link
Contributor

ockham commented Dec 5, 2021

Maybe we should start removing unit tests for APIs that are part of WP core since updates should be submitted to the core anyways.

Yeah, that's the strategy we've followed elsewhere, e.g. #36855

@Mamaduka
Copy link
Member

Mamaduka commented Dec 5, 2021

Thanks for the confirmation, @ockham.

Let's merge this PR since it also contains updates for the actual REST controller and will be helpful for WP 5.8.

We can remove the unit test in a separate PR.

@ramonjd
Copy link
Member Author

ramonjd commented Dec 5, 2021

The plugin has to support older versions of WP, and this is the new API available in 5.9. The WP_REST_URL_Details_Controller class is conditionally loaded. This and unit tests being out of sync is also the reason for test failures.

Thanks for the context and for testing @Mamaduka !! 🙇

I can't merge until the other tests pass, so I'll try to hunt down an admin to merge this since the PHP unit test fails are probably blocking any PRs that attempt to fix the other tests 😅

@ramonjd ramonjd added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta and removed [Type] Bug An existing feature does not function as intended [Type] Question Questions about the design or development of the editor. labels Dec 5, 2021
@noisysocks noisysocks merged commit 8073d19 into trunk Dec 5, 2021
@noisysocks noisysocks deleted the try/update_details_controller_test_pre_site_transient_filter branch December 5, 2021 23:44
@github-actions github-actions bot added this to the Gutenberg 12.2 milestone Dec 5, 2021
@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 6, 2021
noisysocks pushed a commit that referenced this pull request Dec 6, 2021
* Testing whether changing to the pre_site_transient_filter fixes the PHP unit tests, reflecting the change in wordpress/develop 54499

* Use global transients

Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants