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

Spike: Decide how to handle deprecated expectWarning/expectError/expectDeprecation in unit tests #40

Open
GuySartorelli opened this issue Apr 24, 2023 · 1 comment

Comments

@GuySartorelli
Copy link
Member

PHPUnit 9 deprecated expecting PHP errors in unit tests. These expect methods are removed in PHPUnit 10. There is no migration path.
See also discussion in silverstripe/silverstripe-framework#10749 (comment)

  • In some cases we may need to tell PHPUnit to expect some error or warning in order to test the behaviour that happens when such a warning/error is emitted (or else the test will fail, since PHPUnit converts those into exceptions).
  • Some of these warnings/errors may be important to explicitly test (e.g. deprecation warnings).

We need to decide how we will handle this scenario in a consistent and systematic way.

@emteknetnz
Copy link
Member

emteknetnz commented Apr 25, 2023

Based on the linked comment on the phpunit repo it seems like there's really only 2 options:
a) Remove tests for deprecations, notices etc
b) In the unit test, modify the error handler so that E_USER_* are upgraded to throwing exceptions sebastianbergmann/phpunit#5062 (comment)

We'll definitely need to do b) for the Deprecation class

I suspect we'll just make a streamlined way to do b) and upgrade all the existing expect*() calls we make. We'll could add a static withSomething(callable $func) style method to SapphireTest that automatically modifies the error_handler then resets it. Note this might break if something within $func itself modifies the error_handler

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

No branches or pull requests

2 participants