search

doctrine / DoctrineORMModule

Doctrine ORM Module for Laminas
https://www.doctrine-project.org/projects/doctrine-orm-module.html
MIT License
439 stars 229 forks source link

Fixed deprecation on Serializable interface #698

Closed driehle closed 2 years ago

driehle commented 2 years ago

fixes #695

This sets convertDeprecationsToExceptions="true" in PHPUnit configuration, so that in the future we can see failings tests when there are deprecation messages in PHP.

driehle commented 2 years ago

@greg0ire While working on #695 I found that Github actions won't fail unit tests when deprecation messages occur (see https://github.com/doctrine/DoctrineORMModule/runs/4538099900?check_suite_focus=true). Is there a certain reason for this, i.e. is this the intended behaviour?

I think it would make sense to change this. I thought setting convertDeprecationsToExceptions="true" in phpunit.xml.dist would be enough, but it looks like error_reporting=E_ALL has to be set as well. With this, tests are now failing (https://github.com/doctrine/DoctrineORMModule/runs/4539273011?check_suite_focus=true) as I would have expected it.

greg0ire commented 2 years ago

I'd say yes, it's intended: the build should not suddenly fail from one day to the other. I'd say you can change that if you feel up to dealing with deprecations as they occur. Also, I know that in some repositories, we do fail the build if static analysis detects deprecations (which we can ignore or fix on a case by case basis).

It might be a better experience to have a non-required job fail if a previous phpunit job had deprecations, that way you are aware of the issue without it blocking the merge.