Skip to content

Conversation

cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Oct 3, 2025

User description

💥 What does this PR do?

This PR updates the args used when starting Chrome and Edge for internal Ruby tests

  • removes --disable-gpu (this used to be necessary for the old headless mode on Windows)
  • changes --headless=chrome to --headless (old headless mode was removed in Chrome 112, and these args now do the same thing)

🔄 Types of changes

  • Build/Tests

PR Type

Tests


Description

  • Update Chrome/Edge headless argument from --headless=chrome to --headless

  • Remove deprecated --disable-gpu argument for both browsers

  • Modernize browser launch arguments for Ruby test environment


Diagram Walkthrough

flowchart LR
  A["Chrome/Edge Options"] --> B["Remove --disable-gpu"]
  A --> C["Update --headless=chrome to --headless"]
  B --> D["Modernized Browser Args"]
  C --> D
Loading

File Walkthrough

Relevant files
Tests
test_environment.rb
Modernize Chrome/Edge browser launch arguments                     

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

  • Updated headless argument from --headless=chrome to --headless for
    Chrome and Edge
  • Removed deprecated --disable-gpu argument from both browser
    configurations
  • Modernized browser launch arguments for test compatibility
+2/-4     

@selenium-ci selenium-ci added the C-rb Ruby Bindings label Oct 3, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 3, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@cgoldberg cgoldberg added B-build Includes scripting, bazel and CI integrations and removed Review effort 1/5 labels Oct 3, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 3, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Avoid mutable default arguments

Do not use a mutable default array for args; default to nil and initialize
inside the method to avoid unintended state sharing between calls.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb [273-280]

-def chrome_options(args: [], **opts)
+def chrome_options(args: nil, **opts)
+  args = (args || []).dup
   opts[:browser_version] = 'stable' if WebDriver::Platform.windows?
   opts[:web_socket_url] = true if ENV['WEBDRIVER_BIDI'] && !opts.key?(:web_socket_url)
   opts[:binary] ||= ENV['CHROME_BINARY'] if ENV.key?('CHROME_BINARY')
   args << '--headless' if ENV['HEADLESS']
   args << '--no-sandbox' unless Platform.windows?
   WebDriver::Options.chrome(args: args, **opts)
 end

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate inputs and states early to prevent logic errors; avoid mutating shared default arguments across calls.

Low
Avoid mutable default arguments

Replace the mutable default array for args with nil and initialize within the
method to prevent cross-invocation state leakage.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb [282-289]

-def edge_options(args: [], **opts)
+def edge_options(args: nil, **opts)
+  args = (args || []).dup
   opts[:browser_version] = 'stable' if WebDriver::Platform.windows?
   opts[:web_socket_url] = true if ENV['WEBDRIVER_BIDI'] && !opts.key?(:web_socket_url)
   opts[:binary] ||= ENV['EDGE_BINARY'] if ENV.key?('EDGE_BINARY')
   args << '--headless' if ENV['HEADLESS']
   args << '--no-sandbox' unless Platform.windows?
   WebDriver::Options.edge(args: args, **opts)
 end
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Validate inputs and states early to prevent logic errors; avoid mutating shared default arguments across calls.

Low
Possible issue
Restore a flag for headless stability

Conditionally re-add the --disable-gpu flag when running Edge in headless mode
to improve stability and prevent potential rendering issues in CI/CD
environments.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb [282-289]

 def edge_options(args: [], **opts)
   opts[:browser_version] = 'stable' if WebDriver::Platform.windows?
   opts[:web_socket_url] = true if ENV['WEBDRIVER_BIDI'] && !opts.key?(:web_socket_url)
   opts[:binary] ||= ENV['EDGE_BINARY'] if ENV.key?('EDGE_BINARY')
-  args << '--headless' if ENV['HEADLESS']
+  if ENV['HEADLESS']
+    args << '--headless'
+    args << '--disable-gpu'
+  end
   args << '--no-sandbox' unless Platform.windows?
   WebDriver::Options.edge(args: args, **opts)
 end
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion raises a valid point about potential test flakiness by re-adding the --disable-gpu flag for headless runs, but this contradicts the PR's explicit removal of a flag that is officially considered unnecessary with the new headless mode.

Low
  • More

@cgoldberg
Copy link
Member Author

All Ruby tests pass in CI

@cgoldberg cgoldberg merged commit 1394608 into SeleniumHQ:trunk Oct 3, 2025
21 of 22 checks passed
@cgoldberg cgoldberg deleted the rb-update-chrome-test-args branch October 3, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants