Tests: Introduce Reflection API helper methods.

[costdev]

In unit tests, the Reflection API is often used to get or set a private or protected property's value, or to invoke a private or protected method.

To do so, the Reflection<Method|Property>::setAccessible() method must be called with true before use, then should be called again with false afterwards. There are quite a lot of instances in the test suite where the Reflection API is used, and for a decent number of these, accessibility is not reset.

For brevity, much like assertSameSets(), and to ensure that accessibility is always reset, this ticket aims to introduce new methods to the WP_UnitTestCase_Base class:

::reflect_and_invoke( object $obj, string $method, mixed ...$args ) : mixed

• Reflects a private or protected method and invokes it.
• Returns the method's return value.

::reflect_and_get_value( object $obj, string $property ) : mixed

• Reflects a private or protected property and gets its value.
• Returns the property's value.

::reflect_and_set_value() : mixed

• Reflects a private or protected property and sets its value.
• Returns the previous value for convenient resetting.

While this means the creation of new Reflection<Method|Property> objects and two calls to ::setAccessible() for each, I think that this is worthwhile as it helps us have a more robust test suite, with less code to write in test methods. Plus, we could also explore possible caching in future.

[costdev]

Examples

Example 1: Ensure that a private property is set when a private method is called.

What this would normally look like:

<?php
public function test_should_set_queries() {
    // Reflect the property.
    $reflected_property = new ReflectionProperty( self::$instance, 'queries' );

    // Make the property accessible.
    $reflected_property->setAccessible( true );

    // Backup the property's value.
    $queries_backup = $reflected_property->getValue( self::$instance );

    // Reflect the method.
    $reflected_method = new ReflectionMethod( self::$instance, 'do_something' );

    // Make the method accessible.
    $reflected_method->setAccessible( true );

    // Invoke the method.
    $reflected_method->invoke( self::$instance );

    // Make the method inaccessible.
    $reflected_method->setAccessible( false );

    // Get the property's value.
    $actual = $reflected_property->getValue( self::$instance, 'queries' );

    // Restore the property's value.
    $reflected_property->setValue( self::$instance, $queries_backup );

    // Make the property inaccessible.
    $reflected_property->setAccessible( false );

    $this->assertSame( 'expected_queries', $actual );
}

What this could look like:

<?php

public function test_should_set_queries() {
    // Reflect the property and backup its value.
    $queries_backup = $this->reflect_and_get_value( self::$instance, 'queries' );

    // Reflect and invoke the method.
    $this->reflect_and_invoke( self::$instance, 'do_something' );

    // Reflect and get the property's value.
    $actual = $this->reflect_and_get_value( self::$instance, 'queries' );

    // Reflect and restore the property's value.
    $this->reflect_and_set_value( self::$instance, 'queries', $queries_backup );

    $this->assertSame( 'expected_queries', $actual );
}

---

Example 2: Ensure that a private property's value affects a public method's result.

What this would normally look like:

<?php
public function test_queries_should_make_get_something_return_something_else() {
    // Reflect the property.
    $reflected_property = new ReflectionProperty( self::$instance, 'queries' );

    // Make the property accessible.
    $reflected_property->setAccessible( true );

    // Backup the property's value.
    $backup = $reflected_property->getValue( self::$instance );

    // Set the property's new value.
    $reflected_property->setValue( self::$instance, 'new value' );

    // Invoke the method.
    $actual = self::$instance->get_something();

    // Restore the property's value.
    $reflected_property->setValue( self::$instance, $backup );

    // Make the property inaccessible.
    $reflected_property->setAccessible( false );

    $this->assertSame( 'expected result', $actual );
}

What this could look like:

<?php

public function test_queries_should_make_get_something_return_something_else() {
    // Reflect the property, set its new value and get its previous value.
    $backup = $this->reflect_and_set_value( self::$instance, 'queries', 'new value' );

    // Invoke the method.
    $actual = self::$instance->get_something();

    // Reflect and restore the property's previous value.
    $this->reflect_and_set_value( self::$instance, 'queries', $backup );

    $this->assertSame( 'expected result', $actual );
}

[hellofromTonya]

Moving to 6.5, as 6.4 RC1 is in a few hours and then will be branched.

[hellofromTonya]

Some initial thoughts about the proposal (not yet at the PR implementation):

I support adding this test API. Make it easier for contributors while keeping consistency and resetting values and visibility after testing.

I think it can be improved and possibly extended for things like resetting and helpers for static.

Naming:

I'm thinking the reflect_and_* naming could be confusing for contributors. Instead, using the PHP class name could help to align it for better understanding and ease of recognizing its relation to the PHP Reflection API:

• get_reflectionproperty_value()
• set_reflectionproperty_value()
• invoke_reflectionmethod()

Extending:

Statics are handled slightly differently than objects. Might be good to extend the API to include statics such as get_static_reflectionproperty_value().

Resetting:

Instantiating and resetting visibility and values with each API method call might get too heavy for the test suite performance. Instead, @costdev have you considered storing the details and then resetting (visibilities and values) at tear_down()?

[costdev]

@hellofromTonya
Naming:
get_reflectionproperty_value()
set_reflectionproperty_value()
invoke_reflectionmethod()

Sounds good to me!

Extending:
Statics are handled slightly differently than objects. Might be good to extend the API to include statics such as get_static_reflectionproperty_value()

Yes this will need some testing and consideration for sure. I agree that it makes sense to include it in this API.

Resetting:
Instantiating and resetting visibility and values with each API method call might get too heavy for the test suite performance. Instead, @costdev have you considered storing the details and then resetting (visibilities and values) at tear_down()?

I did consider this, though I was also mindful of memory usage in the test suite (which is additionally worse when running xDebug and coverage reports, for example), where storing objects would increase memory usage. Based on this, the example implementation in ​PR 5338 calls unset() on each ReflectionProperty/ReflectionMethod when it's done with them for that call.

I'm not sure whether we're best storing the objects and resetting in tear_down() when we're definitely done with them to improve timing, or calling unset() when we're potentially done with them to reduce the chance of memory limits being hit on contributor's local machines. There's a trade-off either way I think.

I do wonder if it's possible that we're considering optimization too early, and we might be best committing an initial API, then iterating. Having this already committed in the Core test suite may make analyzing and optimizing its implementation somewhat easier, without having to bundle the API and optimization in one PR (or more if we test different approaches). What do you think Tonya? Handle it with the initial implementation, or iterate when we have more data?

[desrosj]

I'm going to punt this since there has been no discussion during this cycle. I'm moving to Future Release, but if someone has the availability to own this during an upcoming release, please do move it to a numbered milestone.

@costdev do you think that this would benefit from a proposal on Make Core? Maybe we could drum up some feedback from the community.