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

Don't deprecate setExpectedException(), this will cause issues. #2074

Closed
schlessera opened this issue Feb 12, 2016 · 6 comments
Closed

Don't deprecate setExpectedException(), this will cause issues. #2074

schlessera opened this issue Feb 12, 2016 · 6 comments

Comments

@schlessera
Copy link

I just read the article https://thephp.cc/news/2016/02/questioning-phpunit-best-practices, and cannot really comprehend the conclusion.

I applaud the move to a better structure to test exceptions. And I have always used the setExpectedException() instead of the annotation, because of some of the reasons you have mentioned.

However, in the article, the conclusion is that setExpectedException() will be deprecated for PHPUnit 6, and the annotation will stay.

This means that I cannot use the new functions because of the PHP version requirements, and can't continue to use setExpectedException() because it will become deprecated... so you basically force me to instead use the annotation now for the foreseeable future.

I think that there's no harm in keeping the setExpectedException() method for now, and it can be rewritten as a simple stub that uses the new methods for better maintainability. So please don't deprecate it (just yet).

@sebastianbergmann
Copy link
Owner

The setExpectedException() method has already been deprecated and will be removed in PHPUnit 6.

@alexislefebvre
Copy link

@schlessera If I understood the article correctly, you have to replace setExpectedException() with expectException(), see the end of the article:

Hindsight is easier than foresight. I am not sure anymore that adding the @expectedException did more good than harm. I do not currently plan to remove it from PHPUnit, though. But going forward I will recommend using expectException() etc. for testing exceptions.

So the annotation will stay and Sebastian recommend to use expectException(), you're not forced to use the annotation.

@stof
Copy link
Contributor

stof commented Feb 16, 2016

@schlessera deprecating a function means that it still works but you should consider migrating to the new way. So there is no harm there. You will simply need to migrate your tests to the new methods before upgrading to PHPUnit 6 in 2017

@schlessera
Copy link
Author

@alexislefebvre Yes, thank you. I understood that. The issue for me is that the new expectException() is PHPUnit 5.2+, which makes it PHP5.6+.

A number of my clients are still on PHP 5.5, so I'm currently running all my code with PHPUnit 4.8. I would prefer to have all my code on PHP7, but I don't always control the hosting environment of my clients.

@schlessera
Copy link
Author

@stof Yes, I will probably stay with setExpectedException() for now and keep the roadmap in mind for the future. Hopefully, PHP7 will bring new motivation to hosting companies to update their environment, as they can save resources.

@alexislefebvre
Copy link

@schlessera You can create a class which inherit from \PHPUnit_Framework_TestCase and add a expectException() function which call setExpectedException(). Then use this class in your tests. This way you will write code ready for PHP7 and it will work on PHP 5.3 too. This is sort of backporting expectException to PHPUnit <5.2.

Example (not tested):

class MyTestCase extends \PHPUnit_Framework_TestCase
{
    public function expectException($exception)
    {
        self::setExpectedException($exception);
    }
}

In your tests:

class MyClassTest extends MyTestCase
{
    // ...
    // Call the alias function
    $this->expectException('Exception');
}

pdt256 added a commit to inklabs/kommerce-core that referenced this issue Jun 14, 2016
DarkHorse0725 added a commit to DarkHorse0725/Kommerce that referenced this issue Dec 18, 2022
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

No branches or pull requests

4 participants