Skip to content

Fix unit tests#172

Merged
swissspidy merged 8 commits intomasterfrom
fix/unit-tests
May 22, 2024
Merged

Fix unit tests#172
swissspidy merged 8 commits intomasterfrom
fix/unit-tests

Conversation

@swissspidy
Copy link
Member

@swissspidy swissspidy commented May 22, 2024 •
edited
Loading

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 •
edited
Loading

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

php-cli-tools/tests/bootstrap.php

Line 22 in 6507ba2

spl_autoload_register( 'cli_autoload' );

@thelovekesh
Copy link
Member

thelovekesh commented May 22, 2024

Can I send some tests commits to this PR?

@swissspidy
Copy link
Member Author

swissspidy commented May 22, 2024

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

thelovekesh commented May 22, 2024

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

@thelovekesh
Copy link
Member

thelovekesh commented May 22, 2024 

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

@swissspidy
Copy link
Member Author

swissspidy commented May 22, 2024

That was it! Thanks @thelovekesh!

@thelovekesh
Copy link
Member

thelovekesh commented May 22, 2024

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

swissspidy commented May 22, 2024

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

Reviewers

@thelovekesh thelovekesh thelovekesh approved these changes

Assignees

No one assigned

Labels

scope:testing

Projects

None yet

Milestone

0.12.0

Development

Successfully merging this pull request may close these issues.

2 participants

@swissspidy @thelovekesh