Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8 Support with PHPUnit ^8.5 (9.3 wouldn't work) #652

Closed
wants to merge 3 commits into from
Closed

PHP 8 Support with PHPUnit ^8.5 (9.3 wouldn't work) #652

wants to merge 3 commits into from

Conversation

thadbryson
Copy link

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related Issue
Need Doc update no

Describe your change

Provides PHP 8 Support with PHPUnit working up until ^9.3. I have not been able to test with PHP ^5.3. And I haven't been able to test the following:

  • Algolia\AlgoliaSearch\Tests\Integration\MultiClusterManagementTest::testMultiClusterManagement
  • Algolia\AlgoliaSearch\Tests\Integration\PersonalizationStrategyTest::testPersonalizationStrategy

I don't have those features on my Algolia account.

I'm not sure if PHPUnit ^9.3 is better over 8.5. I may be able to change to 8.5

What problem is this fixing?

PHP 8 support for this project is blocking PHP 8 support for laravel/scout and Laravel Nova.
I would of edited the code for #644 pull request but couldn't. I thank that author for the code I used.

@thadbryson
Copy link
Author

OK...so PHPUnit 9 did not work out. Going with 8.5. Will try that.

@driesvints
Copy link

This won't work, not even with PHPUnit since the new void return type will fail on anything below PHP 7

@thadbryson
Copy link
Author

Yeah I'm working on using PHPUnit 8.5. I removed the void return types. I got it working except for Guzzle and the tests I mentioned where my Algolia account doesn't have access.

@thadbryson thadbryson changed the title PHP 8 Support with PHPUnit ^9.3 PHP 8 Support with PHPUnit ^8.5 (9.3 wouldn't work) Dec 10, 2020
@driesvints
Copy link

@thadbryson this will never work because Algolia needs support all the way from PHP 5.3 til PHP 8

@thadbryson
Copy link
Author

Yeah I just saw PHP-CS-Fixer doesn't support the 5.3 to 8 range. I bumped the version of it in my composer.json but can't test it on 5.3. I'll have to cancel this pull request. I agree with @driesvints that they'll have to separate versions to support PHP 8.

@thadbryson thadbryson closed this Dec 10, 2020
@thadbryson thadbryson deleted the php8 branch December 10, 2020 14:39
@greg0ire
Copy link
Contributor

@driesvints @thadbryson possible solution for the : void issue: https://twitter.com/s_bergmann/status/1333483559181574148 ?

@driesvints
Copy link

@greg0ire unfortunately that won't work either. This entire test suite is so old that there's much more issues than just the void return type. We simply can't use newer PHPUnit versions with the current test suite and the current PHPUnit v4 fails on PHP 8. Updating the entire test suite won't allow it to run on PHP <7.? anymore. We're in a stalemate where only a new major version with the dropping of older PHP versions will be a path forward.

I'm more than happy to be proven wrong though if anyone's up for a different solution.

@thadbryson
Copy link
Author

I think it'll need a v3 like @driesvints has suggested. Supporting PHP ^7.1 and PHPUnit ^9.3. I'm working on that now.

@greg0ire
Copy link
Contributor

I'm more than happy to be proven wrong though if anyone's up for a different solution.

No, it was just in case : void was the only issue, but if there are a lot more issues then dropping support for PHP < 7.1 is probably the most sensible solution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants