Skip to content

[rb] [test] Log level trace in test environment#17401

Draft
nvborisenko wants to merge 1 commit intoSeleniumHQ:trunkfrom
nvborisenko:rb-test-trace-level
Draft

[rb] [test] Log level trace in test environment#17401
nvborisenko wants to merge 1 commit intoSeleniumHQ:trunkfrom
nvborisenko:rb-test-trace-level

Conversation

@nvborisenko
Copy link
Copy Markdown
Member

Try to "fix" CI.

💥 What does this PR do?

This pull request makes a small adjustment to the Firefox WebDriver setup for debugging. The change updates the argument passed to the Firefox driver when debug logging is enabled, switching from the --log trace flag to the -vv flag.

  • test_environment.rb: Updated the debug logging argument for the Firefox WebDriver from --log trace to -vv for improved compatibility or verbosity.

🔧 Implementation Notes

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Cleanup (formatting, renaming)

Copilot AI review requested due to automatic review settings April 28, 2026 11:45
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Update Firefox WebDriver debug logging flag to -vv

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Update Firefox WebDriver debug logging flag from --log trace to -vv
• Improves compatibility with geckodriver's verbose output flag
• Maintains consistent debug logging behavior across test environment

Grey Divider

File Changes

1. rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb 🐞 Bug fix +1/-1

Update Firefox debug logging flag to -vv

• Changed Firefox driver debug logging argument from --log trace to -vv
• Uses more compatible geckodriver verbose flag for improved debugging
• Maintains conditional debug logging when WebDriver logger is at debug level

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 28, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. SE_DEBUG adds extra -vv 🐞 Bug ☼ Reliability
Description
TestEnvironment#firefox_driver appends -vv whenever WebDriver.logger.debug? is true; since
SE_DEBUG forces the logger into debug mode, this makes geckodriver always receive -vv during
SE_DEBUG runs. At the same time Firefox::Service already appends -v under SE_DEBUG, so
geckodriver is launched with both verbosity flags, undermining the service’s intended
SE_DEBUG-controlled logging behavior.
Code

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[284]

+          service.args << '-vv' if WebDriver.logger.debug?
Evidence
The PR change adds -vv based on WebDriver.logger.debug?. In Ruby, SE_DEBUG forces the Selenium
logger into debug mode, so WebDriver.logger.debug? becomes true whenever SE_DEBUG is set.
Independently, the Firefox service implementation also appends -v when SE_DEBUG is present. The
repo’s CI rerun helper explicitly reruns with SE_DEBUG=true, so this combined -v + -vv
behavior will occur in that workflow.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[282-287]
rb/lib/selenium/webdriver.rb[97-104]
rb/lib/selenium/webdriver/firefox/service.rb[29-41]
scripts/github-actions/rerun-failures.sh[21-31]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rb/spec/.../test_environment.rb` adds `-vv` whenever `WebDriver.logger.debug?` is true. Because `SE_DEBUG` forces the logger into debug mode and `Firefox::Service` already adds `-v` under `SE_DEBUG`, SE_DEBUG runs end up launching geckodriver with both `-v` and `-vv`.

### Issue Context
- `SE_DEBUG` is used by the CI rerun script and also changes Ruby logger behavior.
- Firefox service already manages geckodriver verbosity under `SE_DEBUG`.

### Fix Focus Areas
- rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[282-287]
- rb/lib/selenium/webdriver.rb[97-104]
- rb/lib/selenium/webdriver/firefox/service.rb[29-41]

### Suggested change
In `firefox_driver`, only append `-vv` when `WebDriver.logger.debug?` is true **and** `ENV.key?('SE_DEBUG')` is false (or when `service.args` does not already contain a `-v`/`-vv` style flag).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts Firefox/geckodriver debug logging configuration in the Ruby integration test environment by changing the argument used to enable verbose output.

Changes:

  • Update firefox_driver service arguments to use -vv when debug logging is enabled (replacing --log trace).

def firefox_driver(service: nil, **)
service ||= WebDriver::Service.firefox
service.args.push('--log', 'trace') if WebDriver.logger.debug?
service.args << '-vv' if WebDriver.logger.debug?
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

WebDriver.logger.debug? becomes true when SE_DEBUG is set (rb/lib/selenium/webdriver.rb:98). In that case this line adds -vv, but Firefox::Service also appends -v under SE_DEBUG (rb/lib/selenium/webdriver/firefox/service.rb:36-39) and only strips --log... args. Result: with SE_DEBUG you now pass both -vv and -v (and you no longer trigger the intended “overriding user-specified driver logging settings” warning).

Consider keying this off ENV['DEBUG'] instead of WebDriver.logger.debug?, or skip adding -vv when ENV['SE_DEBUG'] is set / when a -v* arg is already present.

Suggested change
service.args << '-vv' if WebDriver.logger.debug?
verbose_arg_present = service.args.any? { |arg| arg.match?(/\A-v+\z/) }
service.args << '-vv' if WebDriver.logger.debug? && !ENV['SE_DEBUG'] && !verbose_arg_present

Copilot uses AI. Check for mistakes.
@nvborisenko nvborisenko marked this pull request as draft April 28, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants