damianszczepanik / cucumber-reporting

HTML reports for Cucumber
GNU Lesser General Public License v2.1
546 stars 402 forks source link

Use only slf4j (excluding legacy commons-logging*) and remove logging manipulations during tests as not needed. #1147

Closed hazendaz closed 8 months ago

hazendaz commented 9 months ago

Tests manipulating logs appeared to me as being done to make less output show up on the console, achieved the same thing by simply using slf4j-simple in test scope and changing what gets printed out.

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (4b5358a) 98.27% compared to head (3f2a4fa) 98.27%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1147 +/- ## ========================================= Coverage 98.27% 98.27% + Complexity 567 565 -2 ========================================= Files 55 55 Lines 1214 1214 Branches 105 105 ========================================= Hits 1193 1193 Misses 10 10 Partials 11 11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

damianszczepanik commented 9 months ago

Question is: what happens when this change is left unmerged?

hazendaz commented 9 months ago

Continues to make users exclude the commons logging library. We actually force that via a super pom using enforcer plugin.

With the logging config file I added, you should not incur any additional logging nor cost for the logging as slf4j doesn't log or construct the logger if it's not in the right logging level. The change to error then blocked all that info which I agree was costly and not useful during tests. I failed to catch timings before and after but it felt the same and output on console was the same. I can get some timings on this change. I believe it's likely a wash.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Damian Szczepanik @.> Sent: Wednesday, January 3, 2024 3:01:14 PM To: damianszczepanik/cucumber-reporting @.> Cc: Jeremy Landis @.>; Author @.> Subject: Re: [damianszczepanik/cucumber-reporting] Use only slf4j (excluding legacy commons-logging*) and remove logging manipulations during tests as not needed. (PR #1147)

Question is: what happens when this change is left unmerged?

— Reply to this email directly, view it on GitHubhttps://github.com/damianszczepanik/cucumber-reporting/pull/1147#issuecomment-1875898320, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODIYAHWWXVRJWYLQ4HLTYMW2IVAVCNFSM6AAAAABBJQQUOOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZVHA4TQMZSGA. You are receiving this because you authored the thread.Message ID: @.***>

damianszczepanik commented 9 months ago

Can you resolve conflicts?

hazendaz commented 8 months ago

yes will catch up here in a few days. Thanks.

hazendaz commented 8 months ago

Can you resolve conflicts?

Rebased, fixed merge conflict, bumped slf4j to 2.0.11