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

Add compatibility with PHPUnit 9.3 #118

Closed
wants to merge 1 commit into from
Closed

Add compatibility with PHPUnit 9.3 #118

wants to merge 1 commit into from

Conversation

villfa
Copy link
Contributor

@villfa villfa commented Sep 12, 2020

This is a required step to build the project with PHP8.

@villfa villfa marked this pull request as draft September 12, 2020 14:25
@villfa villfa marked this pull request as ready for review September 12, 2020 14:33
@GrahamCampbell
Copy link
Member

Will conflict with #113.

@villfa
Copy link
Contributor Author

villfa commented Sep 13, 2020

@GrahamCampbell I have reworked the PR hoping this will make the merge easier

@Nyholm
Copy link
Member

Nyholm commented Sep 19, 2020

Could you please rebase this PR?

@Nyholm
Copy link
Member

Nyholm commented Sep 19, 2020

I started working on PHP8 another ago. I see that that PR is using PHPUnit 9.3.10 with the symfony phpunit bridge.

Have a look at #122. What are the pros/cons between that approach?

@@ -1,11 +1,10 @@
<?php

namespace GuzzleHttp\Tests\Promise;
namespace GuzzleHttp\Promise\Tests;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!


class TestCase extends PHPUnitTestCase
{
public function setExpectedException($exception, $message = null, $code = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually the job of Symfony PHPUnit bridge.

@@ -403,7 +397,7 @@ public function testCanCatchAndYieldOtherException()
$promise->otherwise(function ($value) use (&$result) { $result = $value; });
P\Utils::queue()->run();
$this->assertTrue(P\Is::rejected($promise));
$this->assertContains('foo', $result->getMessage());
$this->assertTrue(strpos($result->getMessage(), 'foo') !== false, "'" . $result->getMessage() . " does not contain 'foo'");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertStringContainsString

@villfa
Copy link
Contributor Author

villfa commented Sep 19, 2020

I started working on PHP8 another ago. I see that that PR is using PHPUnit 9.3.10 with the symfony phpunit bridge.

Have a look at #122. What are the pros/cons between that approach?

When I have originally open this PR I wanted to keep it as minimal as possible.
Now that phpunit-bridge has been added I think it's not relevant to maintain this PR anymore.
I'm closing it in favor of yours.

@villfa villfa closed this Sep 19, 2020
@Nyholm
Copy link
Member

Nyholm commented Sep 19, 2020

Yeah, I appreciate that.

This morning when I started with this library I had no plans of creating PHP8 support.

Thank you for you work.

@villfa villfa deleted the compat-phpunit9 branch September 19, 2020 11:53
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