donatj / mock-webserver

Simple mock web server in PHP for unit testing.
MIT License
131 stars 21 forks source link

Update minimum PHP version to 7.1 #40

Closed whikloj closed 2 years ago

whikloj commented 2 years ago

Wondering if this Windows PR might be ready for prime time?

I think with the wide range of PHP versions you are testing you need to include a composer.lock file for the oldest version.

For PHPUnit the requirements changed so the new versions require the overridden methods to have the same signature (which means return types) ala

public function setUp(): void {

So this is required for PHP 7.1 but void is not valid before 7.1.

Seems like (IMHO) it might be time to drop support for extremely out-of-date versions.

Because otherwise this passes your tests.

whikloj commented 2 years ago

Also noting that it appears you are thinking this way in #34

donatj commented 2 years ago

Wondering if this Windows PR might be ready for prime time?

It's not an active project for a couple reasons -

I think with the wide range of PHP versions you are testing you need to include a composer.lock file for the oldest version.

What leads you to believe that?

Also noting that it appears you are thinking this way in #34

Yep, I'm probably going to close this as it's a dupe. I'm curious what lead you to open this?

whikloj commented 2 years ago

What leads you to believe that?

If you try to support old PHPUnit (for PHP 5) and new PHPUnit (for PHP 7|8) with the same classes you end up having one of them complain. The

public function setUp(): void {

is a good example. If you remove the void then new versions complain

Fatal error: Declaration of InternalServerTest::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /Users/whikloj/www/DAM2/mock-webserver/test/InternalServerTest.php on line 7

but you can't use void in old versions of PHP. So you need to lock your PHPUnit version to whatever the code supports.

I'm not a Windows developer either but had a user desire Windows support which (for me) requires a working CI to test it or I'd never know if it was working. I'll see what I can find out there.

Thanks anyways.

whikloj commented 2 years ago

Sorry just realizing you asked a question at the end and I totally missed it.

Yep, I'm probably going to close this as it's a dupe. I'm curious what lead you to open this?

I have a library (https://github.com/whikloj/BagItTools) that uses this library in some of the tests and recently was asked to make some changes to support it on Windows. I also don't deal in Windows development and so wanted to extend the testing matrix to use Windows, but the tests that use your library just hang.