-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Replace calls to console.log with managed loggers #12909
Conversation
0cafba2
to
1ebceb4
Compare
Hey Selenium team! I'm pretty confident about all tests passing, but didn't want to tick that box because I wasn't able to run all of them on my workstation, only by removing When reviewing, please pay extra attention to which loggers I used in each instance. I think my assumptions are grounded, but you never know :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Sorry, not in a place to parse through the JS right now. Which of the logging features in our docs - https://www.selenium.dev/documentation/webdriver/troubleshooting/logging/ |
This PR replaces uses of |
This is why https://www.selenium.dev/documentation/webdriver/troubleshooting/logging/ was not working in all cases for JS, we were not leveraging the logger library. Thanks, @danielrozenberg! |
Thanks! |
Co-authored-by: Sri Harsha <12621691+harsha509@users.noreply.github.com> Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
Description
This PR replaces calls to
console.<loglevel>()
in JS code with (hopefully) appropriate Selenium-managed loggers.This also removes hardcoded logger names in existing managed logger calls with the referenced names (e.g., replacing
logging.getLogger('webdriver')
with (logging.getLogger(logging.Type.DRIVER)
)Motivation and Context
Fixes #12676
(and related instances of the same logging issue that don't have a reported issue on GH)
Types of changes
Checklist