symfony: [PhpunitBridge] Exit code 1 instead of 0 using "quiet" option in 6.1.0

Symfony version(s) affected

6.1.0

Description

Hi,

I’m using this configuration for the Phpunit Bridge: SYMFONY_DEPRECATIONS_HELPER="quiet[]=indirect&quiet[]=other"

Using Symfony 6.0.x I get a 0 exit code, however with Symfony 6.1.0 I get a 1 exit code.

How to reproduce

  • Have some tests trigger a deprecation
  • SYMFONY_DEPRECATIONS_HELPER="quiet[]=indirect&quiet[]=other"
  • Run tests
  • echo $? should return 0 but returns 1

[Edit] Added a simple reproducer here: https://github.com/jmsche/symfony-deprecation-reproducer

To reproduce using the reproducer:

git clone https://github.com/jmsche/symfony-deprecation-reproducer.git
cd symfony-deprecation-reproducer/
composer i
bin/phpunit 
echo $? # outputs 1
git fetch 
git checkout 6.0
composer i
bin/phpunit 
echo $? # outputs 0

Possible Solution

No response

Additional Context

The only way I managed to get a 0 exit code is to set SYMFONY_DEPRECATIONS_HELPER="disabled=1", using the new logFile option makes PHPUnit return 1 as well.

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Reactions: 5
  • Comments: 32 (25 by maintainers)

Commits related to this issue

Most upvoted comments

The problem was introduced in #45923 , before that PR total would default to 999999, now it defaults to 0. Because of that $isFailing is now true so the process exits with 1.

it was buggy in the first place, showing verbose output for every deprecation types as soon as a threshold is reached

And the fix may have introduced the break with the existing code. (only supposition, it will be hard to know where, when and how the behavior has changed and I would be the least qualified to point the line)

I think this issue can be closed too. I will try to make a pr by the end of the week on the doc to make the difference between max and quiet clearer.

Thanks both of you for the investigations and explanations. 😃

Sorry I missed https://github.com/symfony/symfony/issues/46497#issuecomment-1162294706 and I’m late on the last mention. If this was introduced in #45923 or, in general, as part of the ignoreFile implementation, well that was certainly not intentional. I’ll try to catch up with the comments here and look into the code if I can figure out anything.

No need to overthink it 🙂 a note with the following should suffice:

quiet hides details for the specified deprecation types, but will not change the outcome in terms of exit code. That’s what max is for, and both settings are orthogonal.

In terms of UX, I’m starting to think that a quiet_if_successful setting might be handy. Still in terms of UX, there is no explanation of what exactly is responsible for a failure. It would be nice to have something like “exited with code X because there were 5 direct deprecations, and the configured maximum is 4”. Don’t ask me to implement that though, I have a lot on my plate and I remember the phpunit bridge was quite hard to contribute to the few times I tried. 😛 Just… giving ideas in hope somebody here has too much time on their hands.

I think a documentation update might be cool, since this is not obvious. As for whether it is a breaking change, it’s harder to argue when there are no docs. Maybe it initially behaved like I just described, who knows?

Done a bit higher, 21st june last year 😬, so we are trying other ways as none of us seems to understand properly how it should behave, how to fix or what’s going wrong.

https://github.com/symfony/symfony/issues/46497#issuecomment-1162294706

If the fix is to update the documentation, I can take care of that.

@greg0ire Just in case the notification already reached you but the edit wouldn’t:

I found the original one I wanted to point : #48787

And didn’t confused my tabs 😇

Woops… I may have confused different tabs 😬 Sorry for that ping.

@PythooonUser It’s related, the $isFailing variable also affects whether the deprecations should be displayed.

Would someone be able to create a PR with a failing test?