[6.x] GitHub Actions will run test suite in Windows

[derekmd]

#32975 made all tests green in Windows when run locally. This attempts to keep that going forward.

• Add one more GitHub Actions run for PHP 7.4 on Windows Server 2019.

  • The Ubuntu matrix will discover any issues in PHP 7.2 & 7.3.

• Unfortunately tests requiring these external services must be skipped:

  • DynamoDB
  • Memcached
  • MySQL
  • Redis

  GitHub Actions' service containers only work in Linux environments. The Windows build only offers the mysql.exe client but no local server. To setup any of those services, they would be need to be instances external to the virtual environment. See: slow and costly.

• retry() unit tests need an even longer delta check since 100ms is apparently 110.3ms in CI thanks to the Windows' kernel Sleep() function.

3% of the tests (137 of 4938) will be skipped in Windows however pull request feedback for the remaining 97% has value.

[GrahamCampbell]

I wonder if we should do 7.2 prefer lowest as well, on Windows. So we have a highest and lowest test.

[derekmd]

Adding PHP 7.2/prefer-lowest makes FilesystemAdapterTest::testPath() fail because slash-handling was changed in "league/flysystem".

• Github Actions installs league/flysystem (1.0.8) from 2015 Jul 12: thephpleague/flysystem@941a320
• Which doesn't include this 2017 fix: Fix Windows path handling and simplify path normalization. thephpleague/flysystem#751

So a weird hardcoded fix would have to be added to the test (running Windows, have old league/flysystem installed) or that package will have to be bumped "^1.0.8" to "^1.0.34"

[GrahamCampbell]

I think we should bump the minimum version of flysystem to fix that bug. This is the purpose of prefer-lowest. To catch this kind of thing. :)

Please make the change in all composer.json files that require flysystem, such as the filesystem componen's composer.json.

[derekmd]

I discovered another PHP 7.2 on Windows problem. tearDown() after FilesystemTest@testFilesMethod() doesn't immediately delete the temp directory so all subsequent tests fail. I think this is caused by: https://bugs.php.net/bug.php?id=77077

Windows' kernel delays directory deletion so the next FilesystemTest test is run before the previous test's delete directory operation has completed. In setUp() before testCopyDirectoryReturnsFalseIfSourceIsntDirectory(), adding dump(new Filesystem)->files(static::$tempDir, true)) shows fstat output that subdirectory __DIR__.'/tmp/foo' still exists but it's:

• not readable
• not writable
• not a directory (!!)

The PHP script also can't immediately create the directory again while the kernel has its delete operation queued and PHP 7.2 asserts the directory already exists. Making FilesystemTest create __DIR__.'/tmp' only once for the whole class and changing each test to use unique subdirectories (instead of reusing __DIR__.'/tmp/foo' each time) allows the class to pass again.

class FilesystemTest extends TestCase
{
    private static $tempDir;

    /**
     * @beforeClass
     */
    public static function setUpTempDir()
    {
        self::$tempDir = __DIR__.'/tmp';
        mkdir(self::$tempDir);
    }

    /**
     * @afterClass
     */
    public static function tearDownTempDir()
    {
        $files = new Filesystem;
        $files->deleteDirectory(self::$tempDir);
        self::$tempDir = null;
    }

    protected function tearDown(): void
    {
        m::close();

        $files = new Filesystem;
        $files->deleteDirectory(self::$tempDir, $preserve = true);
    }

    // then change each $this->tempDir & mkdir() call that follows

I'm thinking ignorance is bliss when it comes to complete PHP 7.2 support.

[derekmd]

It's passing now in each environment.

Summary:

• Add GitHub Actions job for PHP 7.4 / prefer-stable in Windows Server 2019
• Add GitHub Actions job for PHP 7.2 / prefer-lowest in Windows Server 2019
• Minimum version bump "league/flysystem" from ^1.0.8 to ^1.0.34 from Laravel expecting Fix Windows path handling and simplify path normalization. thephpleague/flysystem#751 (2017 fix) to be in place but FilesystemAdapterTest::testPath() fails when 1.0.8 (from 2015) is installed.
• Rework how FilesystemTest creates and deletes __DIR__.'/tmp/* subdirectories. This fixes running tests on PHP 7.2 & Windows where subdirectories are asynchronously set for deletion in the previous test but mkdir() would fail in the next test because Windows hadn't yet finalized that kernel call. This is fixed by using per-method unique subdirectory names and keeping the same __DIR__.'/tmp' for the class' duration.
  • I made this commit as -> 7.x -> master merge-up happy as possible. Some of those 7.x "cleanup" commits seem to exist only to cause merge conflict hell from supporting 6.x LTS.

[driesvints]

Hey @derekmd. This is looking good. Thanks for that.

Is there any way to consolidate the job into one? I'm not sure how much work that'd be?

runs-on: [ubuntu-latest, windows]

[GrahamCampbell]

Is there any way to consolidate the job into one? I'm not sure how much work that'd be?

It would be an absolute mess. The current layout is the smallest filesize of the config file I think.

[GrahamCampbell]

Windows where subdirectories are asynchronously set for deletion in the previous test but mkdir() would fail in the next test because Windows hadn't yet finalized that kernel call.

I think this issue also affects docker too, so we are also fixing that at the same time. 🚀

[driesvints]

It would be an absolute mess. The current layout is the smallest filesize of the config file I think.

In that case, can we please also test 7.3 on Windows?

[GrahamCampbell]

In that case, can we please also test 7.3 on Windows?

I thought this was already discussed and we wanted to reduce the number of builds?

[GrahamCampbell]

If we want to be comprehensive, we should do all 6 builds on Windows.

[driesvints]

Think it's best that we just test on the stable version of 7.2, 7.3 and 7.4

[GrahamCampbell]

We already have prefer-lowest working, though? It's important we also test that.

[driesvints]

Then let's just test lowest for 7.2 and stable for 7.3 and 7.4