WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 4 months ago

#52579 closed defect (bug) (fixed)

Delete link-manager.zip after running the REST API plugins controller tests

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

Description

After running the PHPUnit tests the untracked DIR_TESTDATA/link-manager.zip file is left in place, creating a dirty working copy. See WP_REST_Plugins_Controller_Test::setup_plugin_download().

This file should be deleted after the tests that use it have run.

Introduced in [48242].

cc @TimothyBlynJacobs

Attachments (2)

52579.patch (661 bytes) - added by rachelbaker 4 months ago.
Removes link-manager.zip file during teardown
52579.1.patch (659 bytes) - added by rachelbaker 4 months ago.
indentation fix

Download all attachments as: .zip

Change History (11)

#1 @TimothyBlynJacobs
5 months ago

I feel like something regressed here, because I don't think it did this when it was initially committed. IIRC, setup_plugin_download copies the zip to the alternate location because after the plugin was installed, the source file was being deleted. I'm not sure why it isn't being deleted anymore. Looking at the call to unpack_package in WP_Upgrader::run it seems like it should be being deleted.

That being said, we could probably add an unlink in tearDown like we're currently doing for the test plugin regardless.

#2 @desrosj
5 months ago

  • Milestone changed from Awaiting Review to 5.8

#3 @desrosj
5 months ago

I had tested this and forgot to report back. Looks like the file was not being removed after being added in the older branches in my testing locally.

#4 @TimothyBlynJacobs
5 months ago

Ack, my mistake for not catching that before committing. Thanks for checking @desrosj!

Looking at it further, I think the culprit may be test_create_item_and_activate_errors_if_no_permission_to_activate_plugin. It sets up the plugin download even though the plugin won't get installed because the user doesn't have permission.

@rachelbaker
4 months ago

Removes link-manager.zip file during teardown

#5 @rachelbaker
4 months ago

  • Keywords has-patch added; needs-patch removed

@TimothyBlynJacobs Yep, that is the culprit. I added removing the link-manager.zip file if it exists to the tearDown() method in 52579.patch

#6 @davidbaumwald
4 months ago

  • Keywords commit added

@rachelbaker 52579.patch looks good. I tested locally, and link-manager.zip is cleaned up after the unit tests now. I think this just needs a slight fix for indentation. Would you like to handle the commit?

@rachelbaker
4 months ago

indentation fix

#7 @rachelbaker
4 months ago

@davidbaumwald Fixed the indentation in the latest patch. Thanks for calling that out. I missed it, and my code editor wasn't configured for PHP.
If you can, please go ahead with the commit.

#8 @davidbaumwald
4 months ago

@rachelbaker Thanks! Working on the commit now.

#9 @davidbaumwald
4 months ago

  • Owner set to davidbaumwald
  • Resolution set to fixed
  • Status changed from new to closed

In 50633:

Build/Test Tools: Cleanup link-manager.zip after REST API tests are finished.

Introduced in [48242], the link-manager plugin is copied from DIR_TESTDATA/plugins to DIR_TESTDATA during the REST API unit tests, but was not cleaned up afterward. This created a "dirty" local working copy. This change unlinks the copied plugin from DIR_TESTDATA after unit tests are completed.

Props johnbillion, TimothyBlynJacobs, desrosj, rachelbaker.
Fixes #52579.

Note: See TracTickets for help on using tickets.