Skip to content

Conversation

Delta456
Copy link
Member

@Delta456 Delta456 commented Aug 25, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Implements #16201 for Java binding

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Enhancement


Description

  • Add --enable-chrome-logs flag to ChromeDriverService arguments

  • Update test cases to verify the new flag inclusion

  • Redirect browser I/O streams for better log management


Diagram Walkthrough

flowchart LR
  A["ChromeDriverService"] --> B["createArgs() method"]
  B --> C["Add --enable-chrome-logs flag"]
  C --> D["Browser I/O streams redirected"]
  E["Test cases"] --> F["Update expected arguments"]
Loading

File Walkthrough

Relevant files
Enhancement
ChromeDriverService.java
Add enable-chrome-logs flag to driver arguments                   

java/src/org/openqa/selenium/chrome/ChromeDriverService.java

  • Add --enable-chrome-logs argument to the createArgs() method
  • Include comment explaining the purpose of suppressing/redirecting
    browser I/O streams
  • Remove empty line for cleaner formatting
+2/-1     
Tests
ChromeDriverServiceTest.java
Update tests for new chrome-logs flag                                       

java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java

  • Update all test assertions to include --enable-chrome-logs in expected
    argument lists
  • Modify logLevelLastWins() and ignoreFalseLogging() test methods
  • Ensure test coverage for the new flag addition
+8/-5     

@selenium-ci selenium-ci added the C-java Java Bindings label Aug 25, 2025
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌

16201 - Partially compliant

Compliant requirements:

  • Add the --enable-chrome-logs flag when starting chromedriver across all bindings.
  • Ensure the flag redirects/suppresses browser I/O streams rather than letting them go to the user's console.

Non-compliant requirements:

  • Add the --enable-chrome-logs flag when starting chromedriver across all bindings.

Requires further human verification:

  • Verify runtime behavior that browser I/O is redirected/suppressed as intended in different environments (CI, local).
  • Confirm whether EdgeDriver needs similar handling or explicit exclusion in Java binding docs/tests.

1234 - Not compliant

Non-compliant requirements:

  • Address click() not triggering JavaScript in link href in Firefox 2.48.x.

5678 - Not compliant

Non-compliant requirements:

  • Investigate/resolve "ConnectFailure (Connection refused)" when instantiating multiple ChromeDriver instances on Ubuntu.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

Adding --enable-chrome-logs unconditionally may alter default logging behavior for all users; validate no unexpected log noise or performance regressions and that it doesn't conflict with existing log path/level flags.

// Suppress or redirect browser I/O Streams
args.add("--enable-chrome-logs");

return unmodifiableList(args);
Compatibility

Ensure --enable-chrome-logs is supported by the targeted ChromeDriver versions; consider guarding by capability/version or documenting minimum required versions.

// Suppress or redirect browser I/O Streams
args.add("--enable-chrome-logs");

return unmodifiableList(args);

Copy link
Contributor

qodo-merge-pro bot commented Aug 25, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@cgoldberg
Copy link
Contributor

Looks like this causes some test failures :(

@Delta456
Copy link
Member Author

Looks like this causes some test failures :(

Only on windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants