phpro / grumphp

A PHP code-quality tool
MIT License
4.11k stars 430 forks source link

Fix broken tests #995

Closed danepowell closed 2 years ago

danepowell commented 2 years ago
Q A
Branch master for features and deprecations
Bug fix? yes/no
New feature? yes/no
BC breaks? yes/no
Deprecations? yes/no
Documented? yes/no
Fixed tickets comma-separated list of tickets fixed by the PR, if any

Tests are broken in HEAD, caused by some Symfony components recently upgrading from 5 to 6.

I fixed the issues in Psalm per advice in these upstream issues:

The bigger issue is a Symfony EventDispatcher deprecated constructor that apparently went unnoticed and was recently completely removed. I'm unable to grok the documentation on how to fix it:

Have you ever considered implementing deprecation scanning or committing your composer.lock and using Dependabot or Violinist to update dependencies? This wouldn't avoid these problems, but it would at least catch them earlier and pinpoint the cause without breaking tests in HEAD.

New Task Checklist:

danepowell commented 2 years ago

Sorry, it looks like the errors I mentioned are actually not happening in automated tests. I have no idea why. I get them when running Psalm locally.

The breakage in automated tests must be due to something else. Although I think the issues I mentioned are still valid and will become a problem.

veewee commented 2 years ago

Thanks for reporting! Will take care of it.

Noticed the event dispatcher thing indeed; It got removed here without any notice https://github.com/symfony/symfony/pull/40468

About the psalm issue : it looks phpunit related, but we are currently not running psalm on the tests. Not sure if it requires fixing.