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 for experimental/block-editor-settings-mobile endpoint #46816

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

geriux
Copy link
Member

@geriux geriux commented Dec 29, 2022

What?

This test adds a basic test for the Block editor settings endpoint used from the mobile apps.

Why?

Sometimes there are changes that affect this endpoint and it's more recently breaking often, this implies different teams working on fixes and making patch releases.

How?

By adding a basic test that checks the endpoint exists and that it returns the expected keys in the response. I didn't check the content of those for now.

Testing Instructions

  • CI check Unit Tests / PHP should pass.
  • Or manually running npm run test:php locally.

Testing Instructions for Keyboard

N/A

Screenshots or screencast

N/A

@geriux geriux added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Dec 29, 2022
@geriux geriux requested a review from fluiddot December 29, 2022 12:29
@geriux geriux requested a review from oandregal December 29, 2022 13:00
@geriux
Copy link
Member Author

geriux commented Dec 29, 2022

Hey @oandregal 👋 I added you as a reviewer for this PR, not sure if you could check it out (no rush), if not, could you suggest who could I ping for it? Thank you!

@oandregal
Copy link
Member

Seeing that it's a bit more involved than usual, I've tagged @anton-vlasenko who has more experience than I do in this area.

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

The unit test looks good to me ✅. However, since it's the first PHP unit test I review, I'd like to defer the approval to someone else.

@geriux
Copy link
Member Author

geriux commented Jan 4, 2023

Hey @anton-vlasenko 👋 let me know if you could take a look or let us know who else could check it out, thank you!

@geriux
Copy link
Member Author

geriux commented Jan 10, 2023

Hey @oandregal 👋 I was wondering if perhaps you might know someone else who could check this PR? Any help to move this forward would be appreciated, thank you!

Copy link
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

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

The code seems fine to me, it doesn't break anything, and it's not supposed to be backported given the mobile REST endpoint is still experimental. Let's ship it as it provides value for you. People can always do a review post-merge and adjust as necessary.

@geriux geriux merged commit e14f3dc into trunk Jan 12, 2023
@geriux geriux deleted the block-editor-settings-mobile-test branch January 12, 2023 14:25
@github-actions github-actions bot added this to the Gutenberg 15.1 milestone Jan 12, 2023
@juanmaguitar juanmaguitar added [Type] Build Tooling Issues or PRs related to build tooling [Package] Block editor /packages/block-editor labels Feb 1, 2023
@anton-vlasenko
Copy link
Contributor

@geriux @oandregal Sorry for not replying. My bad.
I will do better next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants