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

Output deprecations in CI results #10061

Merged
merged 6 commits into from Aug 19, 2021
Merged

Conversation

Seldaek
Copy link
Member

@Seldaek Seldaek commented Aug 18, 2021

This should get us the deprecations right on the CI for PHP 8.1, so easy to keep an eye out for more.

Fixes #10038

/cc @jrfnl

@Seldaek Seldaek added this to the 2.1 milestone Aug 18, 2021
@jrfnl
Copy link
Contributor

jrfnl commented Aug 18, 2021

@Seldaek Thanks for the ping. I'd been using max[self]=0 myself during debugging runs so far.

I think this is definitely a huge improvement.

It still doesn't show enough detail about the actual deprecations (backtrace) to fix them and also still doesn't fail the build on tests with deprecations, but that seems inherent to the Symfony PHPUnit Bridge package, so I guess that's something we just have to accept for now.

Still 97 deprecation notices to fix in PHP 8.1, but at least that's now visible. 👍🏻

@jrfnl
Copy link
Contributor

jrfnl commented Aug 18, 2021

I think I may still have a few more fixes lined up locally, will check and pull what I have and is ready.

@Seldaek
Copy link
Member Author

Seldaek commented Aug 18, 2021

Yeah you still need to run this locally with something like this in phpunit.xml.dist to get full stack traces, but it at least lets one quickly check the CI output to see if there is anything to worry about.

        <env name="SYMFONY_DEPRECATIONS_HELPER" value="/(trim|preg_match|strcmp|strlen|preg_replace|json_decode|strip_tags|is_dir)\(\): Passing null to parameter/"/>

See the last commit 6aa2d15 for something which should help resolve a bunch of the issues, once all tests have been adapter to make use of it. Because the default mock builder returns null by default for undefined calls which tends to cause lots of the current problems.

Another big issue still is places using mocks for Package instances which is probably a bad idea vs just configuring a regular package value object and using that. It also leads to lots of nulls. Anyway have to pack it up for today :)

@Seldaek
Copy link
Member Author

Seldaek commented Aug 18, 2021

And just FYI and to avoid wasting your time, I'll most likely keep going with the test refactorings tomorrow using the above as a base.

@jrfnl
Copy link
Contributor

jrfnl commented Aug 18, 2021

@Seldaek Thanks for the update and the heads-up! I'll hold off looking at anything for now while you're doing this refactor.
And yes, I did notice before that quite some of the remaining issues where to do with MockObjects being used. IIRC that was going to be the next thing for me to look into, so awesome that you're fixing that already now.

@staabm
Copy link
Contributor

staabm commented Aug 18, 2021

You may also raise the phpstans' phpversion, so you get errors including the actual location:

https://phpstan.org/config-reference#phpversion

@Seldaek
Copy link
Member Author

Seldaek commented Aug 18, 2021

What do you mean @staabm ? We run phpstan on php7.4, which is fine, no?

@staabm
Copy link
Contributor

staabm commented Aug 18, 2021

You can configure it to report errors of newer php versions you run it with in the config file.

So you can "enable reporting of errors" which would be emitted by e.g. php 8.1 even when running it with e.g. php7.4

@Seldaek Seldaek merged commit e5a50d1 into composer:master Aug 19, 2021
@Seldaek Seldaek deleted the deprecations branch August 19, 2021 11:14
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

Successfully merging this pull request may close these issues.

PHP 8.1 compatibility
3 participants