Skip to content

Conversation

@swissspidy
Copy link
Member

@swissspidy swissspidy commented May 22, 2024

Trying to fix tests after wp-cli/wp-cli-tests#206

I realized we were not actually running tests against PHP 5.3, only 5.6+, so I figured we can just bump the requirement while at it.

To-do:

  • Figure out why tests are failing on CI, the autoloader doesn't seem to be working anymore... 🤔 Any help appreciated!

@swissspidy swissspidy added this to the 0.12.0 milestone May 22, 2024
@swissspidy swissspidy marked this pull request as ready for review May 22, 2024 19:01
@swissspidy swissspidy requested a review from a team as a code owner May 22, 2024 19:01
@thelovekesh
Copy link
Member

thelovekesh commented May 22, 2024

@swissspidy It seems like on local, tests autoloading has no effect. Tests are running on local even when I disable this line

spl_autoload_register( 'cli_autoload' );

@thelovekesh
Copy link
Member

Can I send some tests commits to this PR?

@swissspidy
Copy link
Member Author

Yes I noticed that too, but no idea why.

And the error message on CI don't seem to make sense either

PHP Warning:  Uncaught require(/home/runner/work/php-cli-tools/php-cli-tools/lib/cli/Table/Ascii.php): failed to open stream: No such file or directory

/home/runner/work/php-cli-tools/php-cli-tools/tests/bootstrap.php:19

/lib/cli/Table/Ascii.php does exist

@thelovekesh
Copy link
Member

Error message have Table in the path while the directory name is table.

@thelovekesh
Copy link
Member

├── table
│   ├── Ascii.php
│   ├── Renderer.php
│   └── Tabular.php

@swissspidy
Copy link
Member Author

That was it! Thanks @thelovekesh!

@thelovekesh
Copy link
Member

Still a mystery to me, why it was not reproducible on my local 🤔. I guess it has something to do with tests custom autoloader.

@swissspidy
Copy link
Member Author

Case insensitive filesystem I suppose

@swissspidy swissspidy merged commit 2d1da47 into master May 22, 2024
@swissspidy swissspidy deleted the fix/unit-tests branch May 22, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants