search

adhocore / php-cli

PHP Console Application made easy- build great console apps with ease. Comes with Zero Dependency and Autocompletion support. Think of it as a PHP cli application framework.
https://github.com/adhocore/php-cli
MIT License
334 stars 35 forks source link

fix dynamic property deprecation notice in testsuite on php 8.3 #103

Closed lubiana closed 1 week ago

lubiana commented 2 weeks ago

ApplicationTest::test_array_action() uses the dynamic property $actionCalled, which is not registered in the testclass. on 8.3 this throws a depecration notice, this small pr fixes that.

lubiana commented 1 week ago

So I tried to figure out, why the pipeline fails, althoug the tests run locally without problem. It seems that the Terminal::width() method returns 0 in the github action runner, which causes the justify method to not return any dashes and therefore the tests fail.

I was able to make that testcase work again: https://github.com/adhocore/php-cli/commit/c90400b76964e49267c78b9fcaac5bebc227b3eb but i am not sure if that is the correct direction as i had to use reflection, to change ther Writers terminal property into a mocked terminal. I think the better solution would be to make the terminal property injectable via constructor, so we can provide a mocked instance in the test. On the other hand the failing test should not be concerned about the terminal class working properly so my solution for the test might be ok, and the other issues could be dealt with in the future.