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 Tests: More robustness, PluginFactory, improved Multisite handling #8792

Merged
merged 81 commits into from
Sep 6, 2021

Conversation

swissspidy
Copy link
Collaborator

@swissspidy swissspidy commented Aug 25, 2021

Context

Summary

This PR adds a single PHPUnit run that is randomly ordered.

This can detect scenarios where the test makes assumptions that are only fulfilled by global state manipulated by a previous test, but are not true in the actual logic to be tested.

In case such a randomly ordered test run fails, the random seed can be found in the GHA log so that this particular ordering can be replicated locally.

Relevant Technical Choices

Add a randomly ordered test run to the GHA workflow

Adds a single PHPUnit run that is randomly ordered.

This can detect scenarios where the test makes assumptions that are only fulfilled by global state manipulated by a previous test, but are not true in the actual logic to be tested.

In case such a randomly ordered test run fails, the random seed can be found in the GHA log so that this particular ordering can be replicated locally.

Note: This PR also fixes some tests that failed during these randomly ordered test runs, but not all. There are simply too many issues with the tests, which requires another big overhaul.

Adds new HasSiteSetup and HasSiteRemoval interfaces

Eliminates the procedural setup_new_site() code by moving logic attached to site setup/teardown in Multisite to the individual services. Basically implementing #7494

Added bonus: rewrite rules are no longer flushed twice upon plugin activation (once by the service container and once by setup_new_site().

Renames existing similar interfaces accordingly:

  • PluginActivationAware
  • PluginDeactivationAware

PluginFactory

Replaces get_plugin_instance() with a dedicated PluginFactory and a simple bootstrap_plugin() function which is hooked to plugins_loaded.

Before that, the plugin was executed right away and not attached to any particular action.

Link_Controller / Hotlink_Controller

Fixes a bug in Link_Controller which unexpectedly caused an external HTTP request.

Slightly refactored both classes to be more aligned and use wp_http_validate_url() for URL validation.

Updates tests accordingly.

To-do

User-facing changes

N/A

Testing Instructions

  • This is a non-user-facing change and requires no QA

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #

@swissspidy swissspidy added Type: Infrastructure Changes impacting testing infrastructure or build tooling Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ Pod: WP & Infra labels Aug 25, 2021
@google-cla google-cla bot added the cla: yes label Aug 25, 2021
@spacedmonkey
Copy link
Contributor

This is the last thing - #8792 (comment) then I will approve.

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

👍

@swissspidy swissspidy merged commit 6fa6812 into main Sep 6, 2021
@swissspidy swissspidy deleted the fix/order-dependent-tests branch September 6, 2021 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants