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

PHPUnit: turns on PHP notices and deprecations #43102

Merged
merged 6 commits into from
Aug 18, 2022

Conversation

hellofromtonya
Copy link
Contributor

@hellofromtonya hellofromtonya commented Aug 9, 2022

Part of #43333

What?

  • Turns on debug constants to allow notices and deprecations to be found and tested.
  • Adds expectNotice() assertion when a notice is expected to happen.
  • Adds test set up for needed to avoid unnecessary notices.
  • Moves testing of invalid datasets (which would throw a notice or deprecation) to a separate test and data provider to clearly identify intent. (further aligns test structures with Core).

This PR brings closer parity to Core which also turns on debug constants such as WP_Debug and uses expectNotice().

Why?

Reasons:

  • Reduce backporting effort by getting tests Core-ready long before backporting starts.
  • Increase parity with WordPress Core's PHPUnit test set up where notices and deprecations are turned on and tested.
  • Promote discovery of potential issues and incompatibilities earlier and continuously through the development and testing cycles.

PHP notices and deprecations provide helpful information for identifying issues and/or code changes.

For example:

  • notices can inform of missing keys or attempting to use a non-array as an array.
  • deprecations alert of code changes that need attention such as data type incompatibilities with PHP native functions, incorrect configuration patterns, etc.

Once backported, Core's tests will fail if there are deprecations or notices thrown, as its test suite turns on debug. By catching these here in Gutenberg's repo, it provides information for resolution before backporting to Core. Why is necessary? In many instances, source code needs changing including adding defensive guarding. Finding these issues during development allows time for investigation and consideration of design intent to properly remedy the issue.

How?

Adds convertDeprecationsToExceptions="true" to php.xml configuration.

Adds the following debug constants to the PHPUnit bootstrap.php file: WP_Debug , LOCAL_WP_DEBUG_LOG, LOCAL_WP_DEBUG_DISPLAY, LOCAL_SCRIPT_DEBUG.

Changes are in the test suite only. Issues within the source code need individual consideration for what the code should do before it attempts to run the code that triggers the notice or deprecation.

FAQ

  • Why not set these constants in .wp-env.json file?

This PR tried that and found that deprecation notices are being thrown in the e2e tests. This PR is focused on PHPUnit only. Adding deprecation expectations to the e2e tests is outside the scope of this PR.

  • Are users affected by notices or deprecations?

Yes and no. Their site will continue running. But unexpected results may happen as the code does not have what it needs to process. The deprecations and notices are identifying where that a problem exists and needs attention.

@hellofromtonya hellofromtonya added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Aug 9, 2022
@hellofromtonya hellofromtonya self-assigned this Aug 9, 2022
@hellofromtonya hellofromtonya force-pushed the fix/phpunit-notices-deprecations branch from 34a6cda to cda782b Compare August 10, 2022 14:42
@hellofromtonya
Copy link
Contributor Author

hellofromtonya commented Aug 10, 2022

UPDATED: 17 Aug 2022: This failing test has been fixed and merged into trunk. This PR was rebased on top of trunk after this fix.

One PHPUnit is failing. Issue #43121 is opened for this issue and PR #43122 resolves the issue.

.............................E......                            477 / 477 (100%)

Time: 2.58 seconds, Memory: [46](https://github.com/WordPress/gutenberg/runs/7769329003?check_suite_focus=true#step:7:47).50MB

There was 1 error:

1) WP_Style_Engine_Test::test_generate_get_styles with data set "invalid_classnames_options" (array(array(array('friends'), array('tasty'))), array(), array())
Undefined index: individual

/var/www/html/wp-content/plugins/gutenberg/packages/style-engine/class-wp-style-engine.php:418
/var/www/html/wp-content/plugins/gutenberg/packages/style-engine/class-wp-style-engine.php:334
/var/www/html/wp-content/plugins/gutenberg/packages/style-engine/class-wp-style-engine.php:558
/var/www/html/wp-content/plugins/gutenberg/packages/style-engine/phpunit/class-wp-style-engine-test.php:37
phpvfscomposer:///var/www/html/wp-content/plugins/gutenberg/vendor/phpunit/phpunit/phpunit:[52](https://github.com/WordPress/gutenberg/runs/7769329003?check_suite_focus=true#step:7:53)

Once fixed, then this PR can be updated with recent trunk and the all CI checks will pass.

* Setup `WP_Block_Supports::$block_to_render`.

Fixes error:
"Trying to access array offset on value of type null"
as `WP_Block_Supports::$block_to_render` is `null` by
default.

* Separates invalid input datasets for testing the notice.

* Adds `$this->expectNotice()` where expected.
@hellofromtonya hellofromtonya force-pushed the fix/phpunit-notices-deprecations branch from cda782b to dd6c556 Compare August 15, 2022 18:34
Deprecation notices are thrown in the e2e tests when setting debug on in the .wp-env.json file. Though these need resolution, adding assertions to expect the deprecations in the e2e tests is outside the scope of this PR.
Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

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

Thanks Tonya! LGTM 🚢

@hellofromtonya hellofromtonya merged commit ca684d4 into trunk Aug 18, 2022
@hellofromtonya hellofromtonya deleted the fix/phpunit-notices-deprecations branch August 18, 2022 10:54
@github-actions github-actions bot added this to the Gutenberg 14.0 milestone Aug 18, 2022
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.

2 participants