CodeIgniter4 icon indicating copy to clipboard operation
CodeIgniter4 copied to clipboard

fix: Tests that failing on Windows

Open pawelkg opened this issue 1 year ago • 1 comments

Description See #7474

A lots of directory separator mismatch in path fixed.

CodeIgniter\Database\Migrations\MigrationRunnerTest::testMigrationUsesSameConnectionAsMigrationRunner ErrorException: unlink(\writable\runner.sqlite): Resource temporarily unavailable \tests\system\Database\Migrations\MigrationRunnerTest.php:477 Propably related to: https://bugs.php.net/bug.php?id=78930

CodeIgniter\HTTP\Files\FileMovingTest::testStore ErrorException: mkdir(): No such file or directory

\system\HTTP\Files\UploadedFile.php:181 \system\HTTP\Files\UploadedFile.php:135 \system\HTTP\Files\UploadedFile.php:355 \tests\system\HTTP\Files\FileMovingTest.php:215

On Windows path was: WRITEPATH . 'uploads/vfs://path/to/file'

In addition change composer test command to work on slower computers.

Checklist:

  • [x] Securely signed commits
  • [ ] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [ ] Unit testing, with >80% coverage
  • [ ] User guide updated
  • [ ] Conforms to style guide

pawelkg avatar Aug 15 '24 12:08 pawelkg

Thank you for sending this PR. But unfortunately It seems that no one is interested in this PR. I cannot review because I don't use Windows.

Also, this PR contains multiple changes, so it is difficult to review. I think it would be more likely to be reviewed if you sent multiple small PRs that address one issue each.

kenjis avatar Aug 26 '24 23:08 kenjis

@kenjis

    function normalize(string $path): string
    {
        return str_replace('\\', '/', $path);
    }

what if I add a function? Apply it in tests to compare paths. I can change all the paths in the code, but it's too much work. Plus it will cause a decrease in performance.

In general, DIRECTORY_SEPARATOR does not matter at all. It can be replaced with /.

Another option is to create a new assertPaths() method and apply normalize() internally

neznaika0 avatar Nov 12 '24 09:11 neznaika0

I would like to try to adapt the work with paths in Windows. To do this, you need to add a couple of functions and update the entire code. There are a few questions:

1. Is it necessary to do this? I have some time for this.

2. How can we better define the global function

function _realpath(string $path): string
{
    return str_replace('\\', '/', realpath($path));
}

before calling Boot.php ? While I'm trying to register it in index.php, spark, Test/bootstrap.php. They are the first to set up paths

3. Another function

function normalize_path(string $path): string
{
    return str_replace('\\', '/', $path);
}

is located in system/Common.php. Works with a path-string.

4. Additionally, we no longer need DIRECTORY_SEPARATOR - all paths are given to Unix. Replace with "/". I consider DS useless as long as there are problems with Windows.

If everything works out, we will be able to guarantee work in XAMPP, laragon...

@michalsn @kenjis @samsonasik @paulbalandan ...I need your opinion. You can open a draft for the feature

neznaika0 avatar Dec 07 '24 17:12 neznaika0

I don't use Windows, so I don't know what the issues people are struggling with here. I have no way to check it or verify it. If these are only problems with the path for testing purposes, then adding assertPath() seems reasonable.

michalsn avatar Dec 12 '24 07:12 michalsn