[logging-too-many-args] Add functional tests for each value of logging-format-style

[Pierre-Sassoulas]

Type of Changes

	Type
✓	🐛 Bug fix

Description

Refs #9118

[DanielNoord]

@Pierre-Sassoulas Do you have more WIP here? Or shall we close this?

[Pierre-Sassoulas]

Creating regression tests is work too, why throw it away ? But, I don't have the fix atm no.

[Pierre-Sassoulas]

All right the I was on mobile and didn't see the code. Now that I read it and the context the intent of the MR is due to this : #9118 (comment)

It was hard to reproduce and the current test is not testing each possibility with each values of logging-format-style=new. Actually the new regression test is 4 lines in 2 files and thus hard to copy paste. I think we should merge and acknowledge that there's a false positive, if someone works on it it's going to make their live easier.

[DanielNoord]

I do think we need to explain why we are mixing old and new style tests in a file that has a docstring of "Tests for logging-too-many-args using old style". That will definitely confuse us later on.

[Pierre-Sassoulas]

It's confusing me right now 😄

[Pierre-Sassoulas]

Updated the docstring for clarity, this is ready for review now.

[Pierre-Sassoulas]

Thanks for the review and thanks for the general cleanup earlier Daniel. I did not check everything but I'm not sure we should loose hope on the "need takes over" label and close them all when they age. Some contributors are giving up really close to completion, there's some value in those MR (but we should close low value or unrecoverable one for sure).

[DanielNoord]

Thanks for the review and thanks for the general cleanup earlier Daniel. I did not check everything but I'm not sure we should loose hope on the "need takes over" label and close them all when they age. Some contributors are giving up really close to completion, there's some value in those MR (but we should close low value or unrecoverable one for sure).

Yeah, those that were close or more recent I kept open. Some of the more controversial ones or low effort ones I closed!

[Pierre-Sassoulas]

Sorry the new comment moved the whole functional test output, had to regenerate 😅

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.82%. Comparing base (4203d87) to head (39038b0).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9121   +/-   ##
=======================================
  Coverage   95.82%   95.82%           
=======================================
  Files         174      174           
  Lines       18810    18811    +1     
=======================================
+ Hits        18024    18025    +1     
  Misses        786      786           

Files	Coverage Δ
pylint/checkers/logging.py	94.73% <100.00%> (+0.03%)	⬆️

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 39038b0