Skip to content

Tests: Core now caches API Endpoint Schemas. Flush that cache. #13245

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.

Sign up for GitHub

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

Jump to bottom
Merged
mdawaffe merged 3 commits into from
Aug 16, 2019
Merged

Tests: Core now caches API Endpoint Schemas. Flush that cache. #13245

mdawaffe merged 3 commits into from
Aug 16, 2019

Conversation

@mdawaffe
Copy link
Member

@mdawaffe mdawaffe commented Aug 16, 2019

Changes proposed in this Pull Request:

As of https://core.trac.wordpress.org/changeset/45811/, Core now caches API Endpoints' Schemas, but does not flush that cache between individual tests.

Our tests sometimes change the expected schema.

To flush the cache, we user Core's method of re-instantiating the entire Controller object.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

No - bug fix.

Testing instructions:

  1. Ensure you're running a version of core Core that includes at least r45811. As of today, the latest nightly works.
    • If you're using the docker image: yarn docker:wp core update --version=nightly
  2. phpunit --group=publicize
    • If you're using the docker image: yarn docker:phpunit --group=publicize

Prior to this patch, you'd see a couple errors about the jetpack_publicize_message key being missing.

After this patch, there are no errors.

Proposed changelog entry for your changes:

None needed? Only affects tests.

@mdawaffe
Publicize: Also test that post_type=post has the publicize message me…
b2fb539 
…ta key.

In addition to testing our Custom Post Types, ensure that normal Posts hav
the `jetpack_publicize_message` key in the response.
@mdawaffe
Tests: Core now caches API Endpoint Schemas. Flush that cache.
b617e04 
As of https://core.trac.wordpress.org/changeset/45811/, Core now caches
API Endpoints' Schemas, but does not flush that cache between individual
tests.

Our tests sometimes change the expected schema.

To flush the cache, we user Core's method of re-instantiating the entire
Controller object.
@mdawaffe mdawaffe added [Feature] Publicize Now Jetpack Social, auto-sharing [Status] Needs Review This PR is ready for review. Unit Tests labels Aug 16, 2019
@mdawaffe mdawaffe added this to the 7.7 milestone Aug 16, 2019
@mdawaffe mdawaffe requested review from a team and roccotripaldi August 16, 2019 20:03
@mdawaffe mdawaffe self-assigned this Aug 16, 2019
@matticbot
Copy link
Contributor

matticbot commented Aug 16, 2019

Caution: This PR has changes that must be merged to WordPress.com
Hello mdawaffe! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D31643-code before merging this PR. Thank you!

@jetpackbot
Copy link
Collaborator

jetpackbot commented Aug 16, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: September 3, 2019.
Scheduled code freeze: August 27, 2019

Generated by 🚫 dangerJS against b6d738d

@mdawaffe
Tests: ->tearDown() should be in reverse order of ->setUp().
b6d738d 
Ensure we do everything in `->tearDown()` in the opposite order of what
happened in `->setUp()`.
@matticbot
Copy link
Contributor

matticbot commented Aug 16, 2019

mdawaffe, Your synced wpcom patch D31643-code has been updated.

roccotripaldi
roccotripaldi approved these changes Aug 16, 2019
View reviewed changes
Copy link
Contributor

@roccotripaldi roccotripaldi left a comment

Choose a reason for hiding this comment

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

Impressive sleuthing here. The tests pass for me locally, once Travis is happy, ship it!

@mdawaffe mdawaffe merged commit a582ab7 into master Aug 16, 2019
@mdawaffe mdawaffe deleted the fix/api-tests-after-r45811-core branch August 16, 2019 20:18
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review This PR is ready for review. labels Aug 16, 2019
This was referenced Aug 16, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@roccotripaldi roccotripaldi roccotripaldi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mdawaffe mdawaffe

Labels

[Feature] Publicize Now Jetpack Social, auto-sharing Touches WP.com Files Unit Tests

Projects

None yet

Milestone

7.7

Development

Successfully merging this pull request may close these issues.

6 participants

@mdawaffe @matticbot @jetpackbot @roccotripaldi @jeherve