Partially add unit tests for WP Upgrader

[jipmoors]

During the Yoast contributor day, me and Karlijn Bok (will add user profile later) have been working on adding unit tests to the WP Upgrader class.

This does not add full coverage so it should not close #53997

PR: ​https://github.com/WordPress/wordpress-develop/pull/1751
Related ticket: #53997

[jipmoors]

Additionally good to note; the total file has around 52% line-coverage due to accidental coverage from other classes.

I have not determined which tests these were - if somebody does; these could be improved by specifying better @covers tags on the methods or classes.

This PR was created during the contributor day at Yoast on october 8th.
The focus of me and ​https://github.com/karlijnbok was to create test coverage for the WP Upgrader classes.

We've chosen the WP_Upgrader class to start off with, as this is a base class for different more specific upgrader classes.

As we both did not have a lot of recent experience in these classes, we decided to start with the simplest methods and work our way up.

We've added some small code adjustments to have better type safety.

The overal line-coverage of the class has increased to 24%.
We did not run branch or path coverage yet.

The restore_temp_backup has not fully been covered yet, running a coverage report will show this.

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

[hellofromTonya]

5.9 Beta 1 is starting in the next 20ish minutes. Didn't get time in the cycle to review the PR properly. Moving to 6.0.

[hellofromTonya]

With 6.0 RC1 tomorrow, moving this ticket to 6.1.

[chaion07]

Thanks, @jipmoors for reporting this. We reviewed this ticket during a recent bug-scrub session. Feedback received from the team:

1. WP_Upgrader Class has additional lines of code which is new. Having the class reviewed would be a good idea.
2. We feel that this ticket needs some attention. So requesting contributors to have a look before the next release if possible.

Cheers!

Props: @cu121 for the comment & @martin.krcho for pointing direction to the right ticket

This PR expands on ​PR 1754 to:

• [x] bring PR 1754 in line with modern Core/Test standards including:
  • [x] improved test method descriptions.
  • [x] clearer test method names.
  • [x] data_ prefixes for data providers per Trac 53119.
  • ​x $message parameters per [Core Handbook: Writing PHPUnit Tests - Using Assertions].
  • [x] line length limits for contributor a11y consideration.
  • ​x filter callbacks changed to closures + static where appropriate per [Core Handbook: Writing PHPUnit Tests - One-off Functions for Hooks].
• ​x adjust a refactor in [PR 1754] to remove a now unnecessary condition and restore a separated if to elseif.
• ​x remove an unnecessary constant added to tests/phpunit/includes/bootstrap.php in [PR 1754].
• [x] remove tests related to the Rollback feature, which is not yet committed.
  • These have also been updated with the above and retained in a separate branch for a later time.
• [x] add missing generic strings to WP_Upgrader::generic_strings() that caused test failures.
• [x] add more tests/datasets.

Line coverage is now 58.04% with trunk.

I believe this is as far as I can go for now with unit tests due to the use of general functions and the need to mock WPDB to ensure the tests are pure unit tests. Otherwise, integration tests can be added to cover the areas not covered by this PR.

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

[costdev]

@hellofromTonya @SergeyBiryukov @audrasjb @ironprogrammer ​PR 3044 is ready for review. If you have time to review, that would be great!

[costdev]

I've updated ​PR3044 to remove some source changes in WP_Upgrader and the related tests.

I'll open a separate ticket and submit a PR if/when ​PR3044 has been committed so that these source changes can be considered in their own context.

[costdev]

#53997 was marked as a duplicate.

[hellofromTonya]

Thank you @costdev for updating the PR. I'm sorry I didn't dive into it for a review during the 6.1 cycle. With 6.1 RC1 is tomorrow, moving this ticket to 6.2. Let's get it reviewed and committed.

[mukesh27]

This ticket was discussed during the recent bug scrub. It looks like it's unlikely that work will be done on this during the 6.2 cycle.

@hellofromTonya Is this still possible to land in 6.2, or should it be moved to Future Release for now?

[hellofromTonya]

Replying to mukesh27:

This ticket was discussed during the recent bug scrub. It looks like it's unlikely that work will be done on this during the 6.2 cycle.

@hellofromTonya Is this still possible to land in 6.2, or should it be moved to Future Release for now?

This is tests only and can be committed at any time during the 6.2 cycle. Let's leave it in the milestone. It's on my TODO list.

[mukesh27]

This ticket was discussed in the bug scrub.

@hellofromTonya Should it be reviewed and ready for committed prior to 6.2 RC1?

Additional props: @costdev

[hellofromTonya]

Thanks for ping @mukesh27. Likely won't have time to get this one in before RC1.

But 6.3 alpha opens next week. Will ensure it gets reviewed and done.