ZIP tests not checking potential clean directory

[SirLouen]

During some tests, completely out of the blue, I found that the tests

test_should_apply_unzip_file_filters
test_should_apply_pre_unzip_file_filters
in both tests/phpunit/tests/filesystem/unzipFilePclzip.php
and tests/phpunit/tests/filesystem/unzipFileZiparchive.php

Were failing

For some reason, if the tests happen to fail, they leave the archive directory undeleted, with root as owner, and since it's not going to handle it anymore future executions of mkdir are going to fail.

Solution: Build a test setup function to check existences and clean directories before running the tests.

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

[aslamdoctor]

I have created a PR for this.

Instructions for testing:

1. Manually create "archive" directory under /tests/phpunit/data/filesystem
2. Update the code in both files where it says $this->assertSame( 1, $filter->get_call_count() ); to $this->assertSame( 2, $filter->get_call_count() );. By doing this, it will intentionally fail the test.
3. Now check the "archive" directory is still there under /tests/phpunit/data/filesystem.
4. It should be removed in any case whether the test fails or passes.

@aslamdoctor
Looks good to me.
But I think it will be better to add cleanup_unzip_destination to tests/phpunit/includes/abstract-testcase.php (always try to DRY)

[aslamdoctor]

Thanks @SirLouen
Requested changes are done now.

Ok, now things are looking good, but it seems that two tests are still failing:

1) Tests_Filesystem_UnzipFilePclzip::test_should_apply_pre_unzip_file_filters
mkdir(): File exists

/var/www/tests/phpunit/tests/filesystem/unzipFilePclzip.php:44

ERRORS!
Tests: 4, Assertions: 3, Errors: 1.

Because we forgot to add the cleanup_unzip_destination in the method test_should_apply_pre_unzip_file_filters.
for both: tests/phpunit/tests/filesystem/unzipFileZiparchive.php
And: tests/phpunit/tests/filesystem/unzipFilePclzip.php

Finally, for PHPCS compliance: Inline comments must end in full-stops, exclamation marks, or question marks
(Just add a full stop by the end of each comment, like: // See #65443 for details.

And with this, I think we are done.

[aslamdoctor]

@SirLouen Ah right, it is updated now.

[SirLouen]

Test Report

Description

This report validates that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8803.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 136.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Reproduction Steps

• Followed these steps

Actual Results

1. ✅ Issue resolved with patch.

[yashjawale]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8803.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: macOS
• Theme: Twenty Twenty-One 2.5
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅ archive folder is deleted when test case passes
2. ✅ archive folder is deleted when test case fails

Supplemental Artifacts

Screencast: ​https://files.catbox.moe/qgz6cb.mov

[sachinrajcp123]

Ensures ZIP tests clean up the destination archive folder before test run. Prevents leftover dir failures.

[kalpeshh]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8803.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 138.0.0.0
• OS: Linux
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Actual Results

1. ✅ Issue resolved with patch and archive folder is deleted even when test fails

I think the ticket reference number has a type, it should be // See #63443 for details instead of // See #65443 for details

[mindctrl]

@aslamdoctor I left some feedback on your PR.

@kalpeshhiran @mindctrl, 6.9 RC1 is scheduled for release next week, so I wanted to check if this pull request can be included. I have committed all of the suggested code changes myself.

[wildworks]

@SergeyBiryukov, The PR seems ready for a commit.

rmdir

Aren't we already using rmdir in the clean_up_unzip_destination

We also need delete_folders with our test case.
Maybe it's a system environment difficulty. But we tested just with rmdir and were reporting errors.
With delete_folders things were solved; this is why we added it.

[SergeyBiryukov]

In 61212:

Tests: Clean up file destination in _unzip_file_*() unit tests.

This ensures clean environment in case of a previous test failure.

Follow-up to [56689].

Props aslamdoctor, wildworks, peterwilsoncc, SirLouen, mindctrl, kalpeshh, yashjawale, sachinrajcp123, SergeyBiryukov.
Fixes #63443.