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

tests: fix PHP deprecated notice - Automatic conversion of false to array #11

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

Ta5r
Copy link
Collaborator

@Ta5r Ta5r commented Sep 16, 2024

Tracking wpengine#283

What

This PR fixes deprecation warnings in several PHPUnit tests caused by accessing an undefined get_option('graphql_general_settings').

Important

This PR is based on wpengine#280 and requires it to be merged first

Why

The deprecated warnings were being shown on running the composer tests.

How

  • Ensure $settings is an array before updating one of it's values.

Screenshot :

Before :
Screenshot 2024-09-16 at 7 11 14 PM

No more deprecation warnings ✅

Screenshot 2024-09-16 at 6 40 47 PM

@Ta5r Ta5r self-assigned this Sep 16, 2024
@justlevine
Copy link
Collaborator

For the screenshot: add a before from running the test on wpengine/main's branch.

@Ta5r
Copy link
Collaborator Author

Ta5r commented Sep 16, 2024

For the screenshot: add a before from running the test on wpengine/main's branch.

Done 👍 ✅

Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

@Ta5r

I rebased on wpengine#280 to remove unnecessary dependencies.

Last step before you can open your PR on wpengine: generate a changeset:

  1. run npm run changeset

  2. The fixes in here are a patch, they're not adding new features/enhancements.

  3. Use conventional commits for the changlog message. In this case we want something along the lines of tests: fix PHP deprecation notices.

@justlevine justlevine changed the title fix : PHP deprecated - Automatic conversion of false to array tests: fix PHP deprecated notice - Automatic conversion of false to array Sep 16, 2024
Comment on lines 14 to 17
$settings = get_option( 'graphql_general_settings' );
if ( ! is_array( $settings ) ) {
$settings = [];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

A more succint way for handling all of these:

Suggested change
$settings = get_option( 'graphql_general_settings' );
if ( ! is_array( $settings ) ) {
$settings = [];
}
$settings = get_option( 'graphql_general_settings', [] );

Copy link
Collaborator

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Ta5r go ahead and open a PR for this branch against wpengine:main, copy over the PR title + description, and then tag me in a comment.

@justlevine justlevine merged commit 1a2d5d6 into main Sep 16, 2024
10 checks passed
@justlevine justlevine deleted the fix/deprecated-test-bugs branch September 16, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants