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

Test enhancement #26

Merged
merged 6 commits into from
Jan 5, 2019
Merged

Test enhancement #26

merged 6 commits into from
Jan 5, 2019

Conversation

peter279k
Copy link
Contributor

Changed log

  • Use expectedException annotation to be compatible with the different PHPUnit version.
  • Use the ::class approach directly in assertInstanceOf expected object.

@andreasschroth
Copy link

Imo, useful changes, although I'd keep the setExpectedException() call explicitly in the code, as it's less explicit in the PHPDOC block.

@peter279k
Copy link
Contributor Author

peter279k commented Aug 10, 2018

@andreasschroth, thank you for reply.
I use the expectedException annotation because the PHPUnit has the different way to call the expected exception method in different versions.

The PHPUnit version is less than 5.7, and it will be the setExpectedException().
When the PHPUnit version is 6.5+, it will be the expectException().

Set the multiple PHPUnit versions is necessary because it let PHPUnit support the multiple PHP versions. You can see more details in official PHPUnit website.

Using the expectedException annotation to be compatible with the different expected exception.

@andreasschroth
Copy link

@peter279k Well okay, thanks for the explanation, that makes absolutely sense. I guess in general it might just make sense to remove PHP 5.x support all together within this library, so it can also make use of the latest features.

@peter279k
Copy link
Contributor Author

@andreasschroth .Thank you for your reply.
I agree with your suggestion. If we remove the php-5.x support, this problem I mention will not be existed.

It can use the expectException method directly and remove the annotation of expected exception.
But I think the owner @ddeboer will be accepted this ,too.

@ddeboer ddeboer merged commit 91281f3 into ddeboer:master Jan 5, 2019
@ddeboer
Copy link
Owner

ddeboer commented Jan 5, 2019

Thanks for the improvements! We should drop PHP 5 support in a separate PR.

@peter279k peter279k deleted the test_enhancement branch January 5, 2019 16:54
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