Prevent sending Location header during test_perflab_aea_clean_aea_audit_action

[westonruter]

Summary

In working on #898, I noticed PHPUnit failing with:

1) Audit_Enqueued_Assets_Tests::test_perflab_aea_clean_aea_audit_action
Cannot modify header information - headers already sent by (output started at /wordpress-phpunit/includes/bootstrap.php:265)

/var/www/html/wp-includes/pluggable.php:1435
/var/www/html/wp-includes/pluggable.php:1545
/var/www/html/wp-content/plugins/performance/modules/js-and-css/audit-enqueued-assets/hooks.php:138
/var/www/html/wp-content/plugins/performance/tests/modules/js-and-css/audit-enqueued-assets/audit-enqueued-assets-test.php:240

This is due to wp_safe_redirect() being called by perflab_aea_clean_aea_audit_action() which is attempting to send the Location header during testing.

It's not clear to me why this unit test failure is not occurring in trunk.

Relevant technical choices

Prevent wp_safe_redirect() from sending the Location header via the wp_redirect filter in the unit test. Capture the redirected URL and test that it is as expected.

Checklist

• PR has either [Focus] or Infrastructure label.
• PR has a [Type] label.
• PR has a milestone or the no milestone label.

[mukeshpanchal27]

Thanks @westonruter for the PR.

The test_ilo_get_normalized_query_vars and test_ilo_can_optimize_response tests change the global state for the URL so can we fixed those test instead of changing this test?

If we add $this->go_to( '' ); at bottom of both tests it will fixed the issue. Could you please take a look it. Thanks!

[felixarntz]

@westonruter This fix makes sense to me. With your changes, the test no longer relies on the global query vars / URL state, but forces a referer. Previously this didn't work correctly since the wp_get_referer() call returned false and thus remove_query_arg() would call the "current" URL.

Separately, I partially agree with @mukeshpanchal27's feedback that we should also ensure the go_to() calls in #898 don't pollute global state, which is potentially the reason this failure was triggered in your branch.

[westonruter]

Separately, I partially agree with @mukeshpanchal27's feedback that we should also ensure the go_to() calls in #898 don't pollute global state, which is potentially the reason this failure was triggered in your branch.

OK, I've fixed this in #898 by unsetting $_SERVER['REQUEST_URI'] in the tear_down method of tests that call go_to. It's bizarre that core doesn't do this automatically (e.g. in clean_up_global_scope or tear_down).

Nevertheless, as Felix notes, this change is still an improvement.