WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 8 days ago

#51669 reopened defect (bug)

Reduce misleading test failures caused by HTTP timeouts, take 2

Reported by: SergeyBiryukov Owned by: SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Background: #44613.

During the 5.5.2 release and subsequent backports all the way to the 3.7 branch, there were quite a few test failures caused by HTTP timeouts. Some examples of these, collected by @desrosj:

1) Tests_HTTP_curl::test_redirect_on_head
Connection timed out after 5001 milliseconds
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

1) Tests_HTTP_curl::test_redirect_on_head
Failed asserting that true is false.

1) Tests_HTTP_curl::test_redirect_on_302
Failed asserting that true is false.

1) Tests_HTTP_Functions::test_get_response_cookies
Failed asserting that an array is not empty.

1) Tests_HTTP_streams::test_redirect_on_301
stream_socket_client(): unable to connect to tcp://api.wordpress.org:80 (Connection timed out)

1) Tests_HTTP_streams::test_no_head_redirections
cURL error 28: Connection timed out after 5000 milliseconds
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

These are already fixed in 4.9 or later branches by [43511], [43512], [46682], and [46996]. These changes should be backported to earlier branches too.

There were, however, some other failures in the REST API plugin controller tests:

1) WP_REST_Plugins_Controller_Test::test_create_item_and_activate_errors_if_no_permission_to_activate_plugin
An unexpected error occurred. Something may be wrong with WordPress.org or this server&#8217;s configuration. If you continue to have problems, please try the <a href="https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.)

1) WP_REST_Plugins_Controller_Test::test_create_item_and_network_activate
Download failed.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

1) WP_REST_Plugins_Controller_Test::test_create_item
An unexpected error occurred. Something may be wrong with WordPress.org or this server&#8217;s configuration. If you continue to have problems, please try the <a href="https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.)

These tests are not in the external-http group. Looking closer, they appear to set up the plugin download to come locally instead of downloading it from WordPress.org, so it's not quite clear why they still attempt to connect to WordPress.org. This will need more investigation.

Attachments (1)

51669.diff (825 bytes) - added by SergeyBiryukov 2 months ago.

Download all attachments as: .zip

Change History (40)

#1 @SergeyBiryukov
4 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 months ago

#3 @SergeyBiryukov
4 months ago

  • Milestone changed from 5.6 to 5.7

The backports are not tied to a particular release, but the second part (plugin controller tests) needs some investigation. Moving to the next release for now.

#4 follow-up: @SergeyBiryukov
3 months ago

  • Description modified (diff)

Just noting that failures/errors in WP_REST_Plugins_Controller_Test also occasionally happen on trunk too, as seen in this run on GitHub Actions for example.

This ticket was mentioned in Slack in #core by sergey. View the logs.


2 months ago

#6 in reply to: ↑ 4 @SergeyBiryukov
2 months ago

  • Keywords has-patch commit added

Replying to SergeyBiryukov:

Just noting that failures/errors in WP_REST_Plugins_Controller_Test also occasionally happen on trunk too, as seen in this run on GitHub Actions for example.

Looking at the stack trace, this is an issue similar to [49491] / #51670:

  • The plugin installation tests added in [48242] / #50321 run WP_REST_Plugins_Controller::create_item().
  • WP_REST_Plugins_Controller::create_item() runs Plugin_Upgrader::install().
  • Plugin_Upgrader::install() runs the upgrader_process_complete action.
  • The action has these four functions attached to it via wp-admin/includes/admin-filters.php:
    • add_action( 'upgrader_process_complete', array( 'Language_Pack_Upgrader', 'async_upgrade' ), 20 );
    • add_action( 'upgrader_process_complete', 'wp_version_check', 10, 0 );
    • add_action( 'upgrader_process_complete', 'wp_update_plugins', 10, 0 );
    • add_action( 'upgrader_process_complete', 'wp_update_themes', 10, 0 );
  • [49491] / #51670 cancels the language pack updates using the async_update_translation filter, but none of the other three functions have a short-circuit like that.
  • Ultimately, this causes external HTTP requests that are unnecessary for plugin installation tests and interfer with the results in case of a timeout.

51669.diff fixes the issue in my testing. This is consistent with WP_Automatic_Updater::run() or Language_Pack_Upgrader::bulk_upgrade, where these hooks are also removed due to not being needed.

#7 @SergeyBiryukov
2 months ago

In 49913:

Tests: Disable update checks while running REST API plugin installation tests.

This prevents external HTTP requests that are not required for the tests in question and may interfere with the results in case of a timeout.

Follow-up to [48242], [49491].

See #51669.

#8 @SergeyBiryukov
2 months ago

Keeping this open for backports:

Last edited 8 weeks ago by SergeyBiryukov (previous) (diff)

#9 @SergeyBiryukov
8 weeks ago

In 49951:

Tests: Set up the plugin download in multisite plugin tests to come locally.

This brings consistency between single site and multisite in REST API plugin installation tests.

Previously, multisite tests were unnecessarily downloading the plugin from WordPress.org on each test run, causing external HTTP requests and leading to failures in case of a timeout.

Follow-up to [48242], [49491], [49913].

See #51669.

#10 @SergeyBiryukov
5 weeks ago

In 50083:

Tests: Disable update checks while running REST API plugin installation tests.

This prevents external HTTP requests that are not required for the tests in question and may interfere with the results in case of a timeout.

Follow-up to [48242], [49491].

Merges [49913] to the 5.6 branch.
See #51669.

#11 @SergeyBiryukov
5 weeks ago

In 50084:

Tests: Disable update checks while running REST API plugin installation tests.

This prevents external HTTP requests that are not required for the tests in question and may interfere with the results in case of a timeout.

Follow-up to [48242], [49491].

Merges [49913] to the 5.5 branch.
See #51669.

#12 @SergeyBiryukov
5 weeks ago

In 50085:

Tests: Set up the plugin download in multisite plugin tests to come locally.

This brings consistency between single site and multisite in REST API plugin installation tests.

Previously, multisite tests were unnecessarily downloading the plugin from WordPress.org on each test run, causing external HTTP requests and leading to failures in case of a timeout.

Follow-up to [48242], [49491], [49913].

Merges [49951] to the 5.6 branch.
See #51669.

#13 @SergeyBiryukov
5 weeks ago

In 50086:

Tests: Set up the plugin download in multisite plugin tests to come locally.

This brings consistency between single site and multisite in REST API plugin installation tests.

Previously, multisite tests were unnecessarily downloading the plugin from WordPress.org on each test run, causing external HTTP requests and leading to failures in case of a timeout.

Follow-up to [48242], [49491], [49913].

Merges [49951] to the 5.5 branch.
See #51669.

#14 @SergeyBiryukov
5 weeks ago

In 50087:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [43511], [43512], [46682], [46996] to the 4.8 branch.
See #51669.

#15 @SergeyBiryukov
5 weeks ago

In 50088:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [43511], [43512], [46682], [46996] to the 4.7 branch.
See #51669.

#16 @SergeyBiryukov
5 weeks ago

In 50089:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.6 branch.
See #51669.

#17 @SergeyBiryukov
5 weeks ago

In 50090:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.5 branch.
See #51669.

#18 @SergeyBiryukov
5 weeks ago

In 50091:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.4 branch.
See #51669.

#19 @SergeyBiryukov
5 weeks ago

In 50092:

Tests: Skip test_readme() if the HTTP request to secure.php.net or dev.mysql.com failed on timeout.

Move skipTestOnTimeout() to WP_UnitTestCase_Base to avoid duplication.

Merges [46682] and [46996] to the 5.3 branch.
See #51669.

#20 @SergeyBiryukov
5 weeks ago

In 50093:

Tests: Skip test_readme() if the HTTP request to secure.php.net or dev.mysql.com failed on timeout.

Move skipTestOnTimeout() to WP_UnitTestCase_Base to avoid duplication.

Merges [46682] and [46996] to the 5.2 branch.
See #51669.

#21 @SergeyBiryukov
5 weeks ago

In 50094:

Tests: Skip test_readme() if the HTTP request to secure.php.net or dev.mysql.com failed on timeout.

Move skipTestOnTimeout() to WP_UnitTestCase_Base to avoid duplication.

Merges [46682] and [46996] to the 5.1 branch.
See #51669.

#22 @SergeyBiryukov
5 weeks ago

In 50095:

Tests: Skip test_readme() if the HTTP request to secure.php.net or dev.mysql.com failed on timeout.

Move skipTestOnTimeout() to WP_UnitTestCase_Base to avoid duplication.

Merges [46682] and [46996] to the 5.0 branch.
See #51669.

#23 @SergeyBiryukov
5 weeks ago

In 50096:

Tests: Skip test_readme() if the HTTP request to secure.php.net or dev.mysql.com failed on timeout.

Move skipTestOnTimeout() to WP_UnitTestCase_Base to avoid duplication.

Merges [46682] and [46996] to the 4.9 branch.
See #51669.

#24 @SergeyBiryukov
5 weeks ago

In 50097:

Tests: Move skipTestOnTimeout() to a more appropriate location, for consistency with other branches.

Follow-up to [50088].

See #51669.

#25 @SergeyBiryukov
5 weeks ago

In 50098:

Tests: Move skipTestOnTimeout() to a more appropriate location, for consistency with other branches.

Follow-up to [50088].

Merges [50097] to the 4.6 branch.
See #51669.

#26 @SergeyBiryukov
5 weeks ago

In 50099:

Tests: Move skipTestOnTimeout() to a more appropriate location, for consistency with other branches.

Follow-up to [50088].

Merges [50097] to the 4.5 branch.
See #51669.

#27 @SergeyBiryukov
5 weeks ago

In 50100:

Tests: Move skipTestOnTimeout() to a more appropriate location, for consistency with other branches.

Follow-up to [50088].

Merges [50097] to the 4.4 branch.
See #51669.

#28 @SergeyBiryukov
5 weeks ago

In 50101:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.3 branch.
See #51669.

#29 @SergeyBiryukov
5 weeks ago

In 50102:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.2 branch.
See #51669.

#30 @SergeyBiryukov
5 weeks ago

In 50103:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.1 branch.
See #51669.

#31 @SergeyBiryukov
5 weeks ago

In 50104:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996] to the 4.0 branch.
See #51669.

#32 @SergeyBiryukov
5 weeks ago

In 50105:

Tests: Reorder WP_UnitTestCase methods for consistency with other branches.

Follow-up to [45013], [50104].

See #51669.

#33 @SergeyBiryukov
5 weeks ago

In 50106:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996], [50105] to the 3.9 branch.
See #51669.

#34 @SergeyBiryukov
5 weeks ago

In 50107:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996], [50105] to the 3.8 branch.
See #51669.

#35 @SergeyBiryukov
5 weeks ago

In 50108:

Tests: Use skipTestOnTimeout() in more HTTP tests.

Adjust it to handle more types of timeouts, e.g. "Resolving timed out", "Connection timed out".

Merges [38757], [43511], [43512], [46682], [46996], [50105] to the 3.7 branch.
See #51669.

#36 @SergeyBiryukov
5 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

#37 follow-up: @SergeyBiryukov
3 weeks ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Got this again when running multisite tests:

There was 1 error:

1) WP_REST_Plugins_Controller_Test::test_create_item
An unexpected error occurred. Something may be wrong with WordPress.org or this server&#8217;s configuration. If you continue to have problems, please try the <a href="https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.)

/var/www/build/wp-admin/includes/plugin-install.php:183
/var/www/build/wp-includes/rest-api/endpoints/class-wp-rest-plugins-controller.php:288
/var/www/build/wp-includes/rest-api/class-wp-rest-server.php:1139
/var/www/build/wp-includes/rest-api/class-wp-rest-server.php:985
/var/www/tests/phpunit/includes/spy-rest-server.php:57
/var/www/build/wp-includes/rest-api.php:479
/var/www/tests/phpunit/tests/rest-api/rest-plugins-controller.php:379

Apparently [49913] and [49951] didn't cover the external HTTP request in plugins_api(). Reopening for investigation.

#38 in reply to: ↑ 37 @SergeyBiryukov
3 weeks ago

Replying to SergeyBiryukov:

Apparently [49913] and [49951] didn't cover the external HTTP request in plugins_api().

Looks like the plugins_api() call in these tests is by design, so they should probably use the external-http group and the skipTestOnTimeout() method.

Last edited 3 weeks ago by SergeyBiryukov (previous) (diff)

#39 @SergeyBiryukov
8 days ago

  • Milestone changed from 5.7 to 5.8
Note: See TracTickets for help on using tickets.