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

Deprecate expect*() methods that have been removed in PHPUnit 10 #5062

Closed
sebastianbergmann opened this issue Sep 25, 2022 · 19 comments
Closed
Assignees
Labels
feature/assertion Issues related to assertions and expectations type/deprecation Something will be/is deprecated
Milestone

Comments

@sebastianbergmann
Copy link
Owner

  • expectDeprecation(), expectDeprecationMessage(), and expectDeprecationMessageMatches()
  • expectError(), expectErrorMessage(), and expectErrorMessageMatches()
  • expectNotice(), expectNoticeMessage(), and expectNoticeMessageMatches()
  • expectWarning(), expectWarningMessage(), and expectWarningMessageMatches()
@sebastianbergmann sebastianbergmann added the type/deprecation Something will be/is deprecated label Sep 25, 2022
@sebastianbergmann sebastianbergmann added this to the PHPUnit 9.6 milestone Sep 25, 2022
@sebastianbergmann sebastianbergmann self-assigned this Sep 25, 2022
@sebastianbergmann sebastianbergmann added the feature/assertion Issues related to assertions and expectations label Sep 25, 2022
@jrfnl
Copy link
Contributor

jrfnl commented Nov 4, 2022

Just wondering where I can find information on what this will be replaced with ?

@sebastianbergmann
Copy link
Owner Author

There are no replacements. PHPUnit 10 no longer converts E_* to exceptions, therefore E_* can no longer be expected.

@InvisibleSmiley
Copy link

The situation is quite confusing right now since version 9.5's expectException suggests to switch to these methods...

sebastianbergmann added a commit that referenced this issue Jan 14, 2023
@thbley
Copy link

thbley commented Feb 3, 2023

@jrfnl I changed my test code like this:

    public function testSendEmailFailure(): void
    {
        set_error_handler(static function (int $errno, string $errstr): never {
            throw new Exception($errstr, $errno);
        }, E_USER_WARNING);

        // phpunit 9.5
        //$this->expectWarning();
        //$this->expectWarningMessage('failed to send');

        // phpunit 10.0
        $this->expectExceptionMessage('failed to send');

        // run some code that produces E_USER_WARNING

        restore_error_handler();
    }

@jrfnl
Copy link
Contributor

jrfnl commented Feb 3, 2023

@thbley Thanks. I think it's good to have such an example in this thread.
It won't help the usecase I was trying to solve, but that's another matter.

otsch added a commit to crwlrsoft/crawler that referenced this issue Feb 6, 2023
Add a forgotten expectation and remove one test. With phpunit 10 you
can't expect PHP errors anymore
(sebastianbergmann/phpunit#5062). But that's
OK, the test wasn't very valuable.
@alexpott
Copy link

alexpott commented Feb 7, 2023

@sebastianbergmann what is the recommended way of testing that an error is triggered in PHPUnit 10? If you want to assert that an E_USER_WARNING is thrown by your code what is the best way to do this? Should you replace the error handler as above or is there a better way?

otsch added a commit to crwlrsoft/crawler that referenced this issue Feb 8, 2023
Add a forgotten expectation and remove one test. With phpunit 10 you
can't expect PHP errors anymore
(sebastianbergmann/phpunit#5062). But that's
OK, the test wasn't very valuable.
hkulekci added a commit to hkulekci/laravel-scout-elasticsearch that referenced this issue Feb 24, 2023
…ann/phpunit#5062 issue. And also, some changes for warning of code structure
@anton-vlasenko
Copy link

anton-vlasenko commented Feb 27, 2023

I just want to comment on the proposed solution above:
In my tests,

restore_error_handler();

is never executed because the exception gets thrown before it.
restore_error_handler() has to be called in the custom error handler instead:

public function test_foo() {
    set_error_handler(
        static function ( $errno, $errstr ) {
            restore_error_handler();
            throw new Exception( $errstr, $errno );
        },
        E_ALL
    );
    
    $this->expectException( Exception::class );
    $this->expectExceptionMessageMatches( 'Expected message' );
    
    // run some code that produces E_USER_WARNING
}

@epitre
Copy link

epitre commented Mar 9, 2023

In my case, I did this :

    public function setUp(): void
    {
        set_error_handler(
            static function ( $errno, $errstr ) {
                throw new \Exception( $errstr, $errno );
            },
            E_ALL
        );
    }

    public function tearDown(): void
    {
        restore_error_handler();
    }

and then, in each test :

$this->expectException(MyException::class);

@BafS
Copy link

BafS commented Mar 13, 2023

Be careful to reset the handler, as @thbley ans @epitre did, some examples don't do it.

It's really unfortunate to have no information about why it got removed and no official explanation about how to upgrade, the upgrade path to v10 is really complex for big codebases.

@jrfnl
Copy link
Contributor

jrfnl commented Mar 13, 2023

@BafS You can find an explanation about the why in the Release announcement: https://phpunit.de/announcements/phpunit-10.html

@BafS
Copy link

BafS commented Mar 13, 2023

@jrfnl thank for the link, great to have something official, I didn't see it mentioned before

@nmeri17
Copy link

nmeri17 commented Mar 27, 2023

@sebastianbergmann what is the recommended way of testing that an error is triggered in PHPUnit 10? If you want to assert that an E_USER_WARNING is thrown by your code what is the best way to do this? Should you replace the error handler as above or is there a better way?

I don't understand why he is uncharacteristically mute and unresponsive to suddenly unplugging a widely used method with neither explanation nor an alternative. If I'm expecting a warning, odds are that that is the subject of the test. The example in the version 10 release notes show further assertion that testSomething returns a value. What is the recommended way for the library developer to verify a code block warns its consumer when certain action is performed? Disabling the action altogether is directly antithetical to my original intention. But this is just a powerless rant since feature is removed already, sadly. I expect the thread to be locked soon

@sebastianbergmann
Copy link
Owner Author

What is the recommended way for the library developer to verify a code block warns its consumer when certain action is performed?

This is a use case that I do not consider worth supporting. I understand that this is frustrating, and I am sorry for that.

@nmeri17
Copy link

nmeri17 commented Mar 27, 2023

This is a use case that I do not consider worth supporting. I understand that this is frustrating, and I am sorry for that.

I guess in my case, the deliberate warning can be relocated to a mockable method. It's a workaround like trying to test register shutdown function. In this case, however, I'm not sure it's possible for all object types and SUTs

jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 30, 2023
Remove the `ExpectPHPException` polyfill and all references to it as support for expecting PHP native and user added deprecations, notices, warnings and errors has been dropped in PHPUnit 10.0.

PHP native Exceptions can still be tested using the `expectException()` method with the name of the PHP native Exception.

Refs:
* https://phpunit.de/announcements/phpunit-10.html
* https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-10.0.md#1000---2023-02-03
* sebastianbergmann/phpunit#3775
* sebastianbergmann/phpunit#5062
* sebastianbergmann/phpunit@a2c784c
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 30, 2023
Remove the `ExpectPHPException` polyfill and all references to it as support for expecting PHP native and user added deprecations, notices, warnings and errors has been dropped in PHPUnit 10.0.

PHP native Exceptions can still be tested using the `expectException()` method with the name of the PHP native Exception.

Refs:
* https://phpunit.de/announcements/phpunit-10.html
* https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-10.0.md#1000---2023-02-03
* sebastianbergmann/phpunit#3775
* sebastianbergmann/phpunit#5062
* sebastianbergmann/phpunit@a2c784c
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 30, 2023
Remove the `ExpectPHPException` polyfill and all references to it as support for expecting PHP native and user added deprecations, notices, warnings and errors has been dropped in PHPUnit 10.0.

PHP native Exceptions can still be tested using the `expectException()` method with the name of the PHP native Exception.

Refs:
* https://phpunit.de/announcements/phpunit-10.html
* https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-10.0.md#1000---2023-02-03
* sebastianbergmann/phpunit#3775
* sebastianbergmann/phpunit#5062
* sebastianbergmann/phpunit@a2c784c
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 31, 2023
Remove the `ExpectPHPException` polyfill and all references to it as support for expecting PHP native and user added deprecations, notices, warnings and errors has been dropped in PHPUnit 10.0.

PHP native Exceptions can still be tested using the `expectException()` method with the name of the PHP native Exception.

Refs:
* https://phpunit.de/announcements/phpunit-10.html
* https://github.com/sebastianbergmann/phpunit/blob/main/ChangeLog-10.0.md#1000---2023-02-03
* sebastianbergmann/phpunit#3775
* sebastianbergmann/phpunit#5062
* sebastianbergmann/phpunit@a2c784c
aloefflerj added a commit to aloefflerj/yet-another-php-controller that referenced this issue Apr 1, 2023
@davisngl
Copy link

davisngl commented Jul 5, 2023

@BafS You can find an explanation about the why in the Release announcement: https://phpunit.de/announcements/phpunit-10.html

There's, unfortunately, no details on what should be the alternative way of testing these things that we would've tested with expect* methods.

@gustavo-olegario-lawnstarter

If you want to keep a similar implementation but also looking for moving on with PHPUnit updates, you can implement this method in your common test class:

protected function assertThrowableMessage(
    string $message,
    callable $callback,
    ...$args
): void
{
    try {
        $callback(...$args);
    } catch (Throwable $e) {
        $this->assertEquals($message, $e->getMessage());
    }
}

This way you can assert the exception was properly thrown during runtime and don't have to deal with the set_error_handler madness.

@davisngl
Copy link

If you want to keep a similar implementation but also looking for moving on with PHPUnit updates, you can implement this method in your common test class:

protected function assertThrowableMessage(
    string $message,
    callable $callback,
    ...$args
): void
{
    try {
        $callback(...$args);
    } catch (Throwable $e) {
        $this->assertEquals($message, $e->getMessage());
    }
}

This way you can assert the exception was properly thrown during runtime and don't have to deal with the set_error_handler madness.

Thank you!
I am just not sure why would their users have to implement something that we use a lot when testing for exceptions.
The reasoning isn't even there, no answer in sight.

@Sopamo
Copy link

Sopamo commented Jul 26, 2023

Just want to add, that for folk who come from google and are using Laravel, you can use the assertThrows method that Laravel provides:

$this->assertThrows(function () use ($news) {
    // Code that should throw
});

There is also an optional second parameter which accepts the class name of the throwable that should get thrown and a third with the error message you expect.

stronk7 added a commit to stronk7/moodle that referenced this issue Aug 4, 2023
This bumps direct dependencies to current ones:

- phpunit: 9.5.x No updates here, there is the 9.6.x series
  available but a lot of deprecation warnings have been
  introduced there without any alternative to use. See:
  - sebastianbergmann/phpunit#5160
  - sebastianbergmann/phpunit#5062
  - ...
  While that will be handy to prepare ourselves to PHPUnit 10
  in some months... we cannot force everybody to jump to 9.6.x
  because that will make a lot of tests to start emitting
  warnings.
  So we stay with PHPUnit 9.5.x for the life of this branch.
- mink-phpwebdriver: 1.2.1 No updates here, just changed the
  constraint because we cannot advance to 1.3.x yet, there is
  a change there causing some app tests to fail. See:
  - oleg-andreyev/MinkPhpWebDriver#81
  So we stay with 1.2.x until that issue is fixed/clarified,
  only then we'll review the status.
- behat: 3.12.x => 3.13.x

And also, automatically, a bunch of 2nd and deepest dependencies.

Generated with php80 that is the lowest php version supported
by this branch and, also, by some of the dependencies, as per
documented @ https://moodledev.io/general/development/tools/composer

Worth mentioning behat/mink-goutte-driver, that we should move
to mink-browserkit-driver, but that's out from this issue scope.
stronk7 added a commit to stronk7/moodle that referenced this issue Aug 4, 2023
This bumps direct dependencies to current ones:

- phpunit: 9.5.x No updates here, there is the 9.6.x series
  available but a lot of deprecation warnings have been
  introduced there without any alternative to use. See:
  - sebastianbergmann/phpunit#5160
  - sebastianbergmann/phpunit#5062
  - ...
  While that will be handy to prepare ourselves to PHPUnit 10
  in some months... we cannot force everybody to jump to 9.6.x
  because that will make a lot of tests to start emitting
  warnings.
  So we stay with PHPUnit 9.5.x for the life of this branch.
- mink-phpwebdriver: 1.2.1 No updates here, just changed the
  constraint because we cannot advance to 1.3.x yet, there is
  a change there causing some app tests to fail. See:
  - oleg-andreyev/MinkPhpWebDriver#81
  So we stay with 1.2.x until that issue is fixed/clarified,
  only then we'll review the status.
- behat: 3.12.x => 3.13.x

And also, automatically, a bunch of 2nd and deepest dependencies.

Generated with php80 that is the lowest php version supported
by this branch and, also, by some of the dependencies, as per
documented @ https://moodledev.io/general/development/tools/composer

Worth mentioning behat/mink-goutte-driver, that we should move
to mink-browserkit-driver, but that's out from this issue scope.
stronk7 added a commit to stronk7/moodle that referenced this issue Aug 5, 2023
This bumps direct dependencies to current ones:

- phpunit: 9.5.x No updates here, there is the 9.6.x series
  available but a lot of deprecation warnings have been
  introduced there without any alternative to use. See:
  - sebastianbergmann/phpunit#5160
  - sebastianbergmann/phpunit#5062
  - ...
  While that will be handy to prepare ourselves to PHPUnit 10
  in some months... we cannot force everybody to jump to 9.6.x
  because that will make a lot of tests to start emitting
  warnings.
  So we stay with PHPUnit 9.5.x for the life of this branch.
- mink-phpwebdriver: 1.2.1 No updates here, just changed the
  constraint because we cannot advance to 1.3.x yet, there is
  a change there causing some app tests to fail. See:
  - oleg-andreyev/MinkPhpWebDriver#81
  So we stay with 1.2.x until that issue is fixed/clarified,
  only then we'll review the status.
- behat: 3.12.x => 3.13.x

And also, automatically, a bunch of 2nd and deepest dependencies.

Generated with php74 that is the lowest php version supported
by this branch and, also, by some of the dependencies, as per
documented @ https://moodledev.io/general/development/tools/composer

Worth mentioning behat/mink-goutte-driver, that we should move
to mink-browserkit-driver, but that's out from this issue scope.
vitormattos added a commit to LibreSign/libresign that referenced this issue Sep 13, 2023
vitormattos added a commit to LibreSign/libresign that referenced this issue Sep 13, 2023
vitormattos added a commit to LibreSign/libresign that referenced this issue Sep 19, 2023
vitormattos added a commit to LibreSign/libresign that referenced this issue Sep 19, 2023
vitormattos added a commit to LibreSign/libresign that referenced this issue Sep 26, 2023
@alexeyp0708
Copy link

alexeyp0708 commented Nov 1, 2023

I always use this approach and don’t have to dance with a tambourine (calling Exception);

$check=false;

set_error_handler(funciton(...$args)use(&$check){
// your code
$check=true;
// your code
},E_NOTICE);
// trigger error your code
restore_error_handler();
unset($check):

$this->assertTrue($check);

You can write your own solution

trait MyAssertionsTrait
{
    /**
     * Will return TRUE if an observed error was thrown, FALSE if there are generated other errors or no errors  , and will throw an error if other errors are observed
     * @param\Closure $call Runs a closure with a code that needs to be checked for an error. If the error is an exception, then the variable $this will be defined in the closure. Checking for $this - isset($this)
     * @param string|\Closure $eql If a line, then checks whether this substring is present at the beginning of the error message.
     * If there is a closure, then the error checking code must be specified in the closure. If the test is successful, the closure should return true.
     * @param int|\Error $level
     */
    public static function isError($call, $eql, $level = E_ALL): bool
    {
        $err_level=is_int($level)?$level:E_ALL;
        $exc_level=is_string($level)?$level:\Error::class;
        if (is_string($eql)) {
            $substr = $eql;
            $eql = function (...$args) use ($substr) {
                if (substr($args[1], 0, strlen($substr)) === $substr) {
                    return true;
                }
                return false;
            };
        }
        $check = false;
        $bef_hand = null;
        $is_caught_error=false;
        $bef_hand = set_error_handler(function (...$args) use (&$bef_hand, &$check, $eql,&$is_caught_error) {
            $is_caught_error=true;
            if (true===$eql(...$args)) {
                // Expected error
                $check = true;
                return true;
            } else if ($bef_hand !== null) {
                //We pass other errors to the previous error handler
                return $bef_hand(...$args);
            }
            return false;
        }, $err_level);
        try{
            $call();
        }catch(\Error $e){
            if(!($e instanceof $exc_level)){
                throw $e;
            }
            $eql=$eql->bindTo($e);
            $code=$e->getCode();
            if(empty($code)){
                $code=E_ERROR;
            }
            if(!$is_caught_error && true===$eql($code,$e->getMessage(),$e->getFile(),$e->getLine())){
                $check=true;
            }
        } finally {
            restore_error_handler();
            return $check;
        }
    }
}    

https://github.com/alexeyp0708/php_phpunit_helpers/blob/master/src/Assertions/AdditionalAssertionsTrait.php

MyAssertionsTrait::isError(); or self::isError() , If you have added a trait to your test class.

$this->assertTrue(self::isError(fn()=>trigger_error('Ops!');,'Ops!',E_USER_NOTICE));

Examples :
https://github.com/alexeyp0708/php_phpunit_helpers/blob/master/tests/Assertions/AdditionalAssertionsTest.php

For the beauty of the design code, you can do this

use MyAssertionsTrait as Assert;
//....
$this->assertTrue(Assert::isError(....));
//...

In any case, you will have to have your own (or shared) repository with assistants to improve testing. In this repository you can collect all the necessary libraries for testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/assertion Issues related to assertions and expectations type/deprecation Something will be/is deprecated
Projects
None yet
Development

No branches or pull requests