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

Log the debuggers found via the logger instead of ConsoleOutput #1258

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

theofidry
Copy link
Member

@theofidry theofidry commented Apr 17, 2020

I'm trying to play with the logs displayed as I think there is a few things that could be improved there. However I believe this small change could add to the noise hence is worth extracting in a separate PR.

As a result here is this PR which is a small tweak on the ConsoleLogger. What does it do?

In 0.16.3:

Screen Shot 2020-05-01 at 22 43 29

In master (due to #1263):

Screen Shot 2020-05-01 at 22 45 22

Now:

Screen Shot 2020-05-01 at 22 45 55

So in other words:

  • the "notice" output (without "IO block") is a very plain message, pretty much what you get with a plain writeln()
  • the other outputs (other log level) are as "normal" – standard Symfony Console

Screen Shot 2020-05-01 at 22 47 34

(since #1263 those are like so):

Screen Shot 2020-05-01 at 22 47 27

@theofidry
Copy link
Member Author

@sanmai @maks-rafalko do you know where I should look at to see how infection resolves which debugger to use?

@maks-rafalko
Copy link
Member

What is a "debugger" in this context? :)

@theofidry
Copy link
Member Author

sorry I meant code coverage generator... Guess I was too tired :D

@sanmai
Copy link
Member

sanmai commented Apr 21, 2020

I don't think it does. This is a testing tool's job. E.g. PHPUnit's.

@maks-rafalko
Copy link
Member

I don't think it does. This is a testing tool's job. E.g. PHPUnit's.

Exactly. I guess it's here

https://github.com/sebastianbergmann/php-code-coverage/blob/28efe6950c2c3ceb86f0fc63aa077617950c78c5/src/CodeCoverage.php#L891-L908

@sanmai
Copy link
Member

sanmai commented Apr 22, 2020

Also note it wasn't like this as of late 2019. Back then XDebug was used over PCOV if both were present.

@theofidry theofidry requested review from maks-rafalko and sanmai and removed request for maks-rafalko May 1, 2020 20:48
@theofidry theofidry marked this pull request as ready for review May 1, 2020 20:48
yield [
LogLevel::DEBUG,
'message',
'[debug] message',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that this test doesn't capture the formatter tags e.g. here the message that we write is <debug>[debug] message</debug> where the tags are handled by the output formatter. I'm don't know however how to test that...

Copy link
Member

@maks-rafalko maks-rafalko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

What do you think about moving

Xdebug detected

below the LOGO?

IMO it's prettier when we have Infection logo first, then all the rest information, as shown here

Screen Shot 2020-05-01 at 22 45 22

Or it's impossible?

@theofidry
Copy link
Member Author

Good idea, done

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.

None yet

3 participants