Tests: Reset global $current_screen between tests to avoid cross-test interdependencies

[hellofromTonya]

As discovered in #52607, the global state of current_screen is not being reset to a known and expected state between tests. Rather, it's left to the individual test that modifies the state to reset it.

Why is this a problem?

If the global state is not consistent when a test starts, it can have unexpected results such as:

• unstable test(s) downstream
• misleading information for a specific piece of functionality under test which could result in "fixing" a problem that doesn't exist in the codebase but instead is in the test suite
• increased effort to debug where the state was changed
• bad contributor experience

Other problems:

• Increases the amount of code in the test suite as each test has to handle setting the state and then resetting it when done
• Inconsistencies in how resetting is done (i.e. in the implementation)
• Inconsistencies in the state's value when resetting (see above for the problems this can cause)
• Reset circuit not being reached when a test fails and the reset is after the failed assertion

This ticket proposes to move the global resetting to the test framework's abstract base test case WP_UnitTestCase_Base and likely in its tearDown method.

Trac ticket: https://core.trac.wordpress.org/ticket/53431

Moves the resetting of current screen globals from individual tests to the base test case.

[hellofromTonya]

​PR 1380 does the following:

• Resets the globals associated with current screen in WP_UnitTestCase_Base::tearDown()
• Removes the resetting from the individual tests (I think I got them all)

How is doing the reset?

There were a few approaches to resetting, but the most popular was:

set_current_screen( 'front' );

What does this do?

• It instantiates a new WP_Screen object
• Sets global $current_screen to the instance
• Sets global $taxnow to an empty string
• Sets global $typenow to an empty string
• Fires 'current_screen' action event passing the instance to callbacks

I wondered: Why is 'front' needed as a starting state? Is it needed?

• Looking at the other resets in the base test case, they are set to null. Tried it for current screen globals after removing resets from individual tests. Tests all passed.
• How about unsetting the globals? Tests all passed.

Alrighty, 'front' isn't needed. Cool.

Which approach did the PR implement?

• Setting the 3 globals to null
• Why? Consistency with the other resets

@SergeyBiryukov @johnbillion What do you think about ​this reset approach?

$current_screen_globals = array( 'current_screen', 'taxnow', 'typenow' );
foreach ( $current_screen_globals as $global ) {
        $GLOBALS[ $global ] = null;
}

[hellofromTonya]

@johnbillion has reviewed and approved. Marking for commit consideration.

[SergeyBiryukov]

In 51419:

Tests: Reset $current_screen global between tests to avoid cross-test interdependencies.

This provides a consistent global starting state for tests that interact with admin screens.

Individual tests no longer need to invoke set_current_screen( 'front' ) (or an alternative implementation) as a reset.

Follow-up to [29251], [29860], [31046], [36721], [38678], [48908], [50433].

Props hellofromTonya, johnbillion.
Fixes #53431.

[prbot]

​hellofromtonya commented on ​PR #1380:

Committed in changeset 51419