Skip to content

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 16, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Log session id to help trace error from CI tests failure that might have

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement, Tests


Description

  • Add session ID logging to CI test output for better debugging

  • Log session ID in autoscaling tests when creating WebDriver sessions

  • Log session ID in Selenium tests setUp methods for all test classes

  • Update browser version lists to latest available versions


Diagram Walkthrough

flowchart LR
  A["WebDriver Session Creation"] -->|"Log session_id"| B["Enhanced Test Output"]
  C["Test Setup Methods"] -->|"Add SessionID to logs"| B
  D["Browser Version Lists"] -->|"Update to latest"| E["Configuration"]
Loading

File Walkthrough

Relevant files
Enhancement
common.py
Add session ID logging to autoscaling tests                           

tests/AutoscalingTests/common.py

  • Modified create_session() function to capture driver instance before
    returning
  • Added print statement to log session ID and browser name when session
    is created
  • Improves debugging capability for autoscaling tests
+3/-1     
__init__.py
Update browser versions and add session ID logging             

tests/SeleniumTests/init.py

  • Updated LIST_CHROMIUM_VERSIONS to include latest versions (140.0,
    139.0, 138.0, etc.)
  • Updated LIST_FIREFOX_VERSIONS to include latest versions (142.0,
    141.0, 140.0, etc.)
  • Added session ID logging to setUp methods in three test classes
  • Enhanced print statements to include SessionID:
    {self.driver.session_id} for better test traceability
+5/-5     
get_started.py
Add session ID logging to get_started tests                           

tests/get_started.py

  • Added session ID logging in run_browser_instance() function
  • Print statement logs session ID and browser name after WebDriver
    creation
  • Improves visibility into session creation for debugging purposes
+1/-0     

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Copy link
Contributor

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 <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
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.

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

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Initialize driver to prevent crash

Initialize self.driver = None at the start of the setUp methods in ChromeTests,
EdgeTests, and FirefoxTests to prevent an AttributeError in tearDown if the
driver fails to initialize.

tests/SeleniumTests/init.py [145-202]

 class ChromeTests(SeleniumGenericTests):
     def setUp(self):
+        self.driver = None
         try:
             options = ChromeOptions()
             options.enable_downloads = SELENIUM_ENABLE_MANAGED_DOWNLOADS
             if not SELENIUM_ENABLE_MANAGED_DOWNLOADS:
                 options.add_argument('disable-features=DownloadBubble,DownloadBubbleV2')
             if TEST_ADD_CAPS_RECORD_VIDEO:
                 options.set_capability('se:recordVideo', True)
             if TEST_CUSTOM_SPECIFIC_NAME:
                 options.set_capability('myApp:version', 'beta')
                 options.set_capability('myApp:publish', 'internal')
 ...
                 )
             start_time = time.time()
             self.driver = webdriver.Remote(
                 options=options, command_executor=SELENIUM_GRID_URL, client_config=CLIENT_CONFIG
             )
             end_time = time.time()
             print(
                 f"Begin: {self._testMethodName} ({self.__class__.__name__}) WebDriver initialization completed in {end_time - start_time} (s) - SessionID: {self.driver.session_id}"
             )
         except Exception as e:
             print(f"::error::Exception: {str(e)}")
             print(traceback.format_exc())
             raise e

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential AttributeError in the tearDown method if driver initialization fails and provides the standard fix, improving test suite robustness.

Medium
Learned
best practice
Guard session_id logging

Guard the log statement to handle potential None or missing session_id right
after creation to avoid confusing logs or attribute errors.

tests/AutoscalingTests/common.py [53-57]

 driver = webdriver.Remote(
     command_executor=CLIENT_CONFIG.remote_server_addr, options=options, client_config=CLIENT_CONFIG
 )
-print(f"Session created: {driver.session_id} ({browser_name})")
+sid = getattr(driver, "session_id", None)
+print(f"Session created: {sid or 'unknown'} ({browser_name})")
 return driver
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Harden shell/logging by avoiding unguarded attribute access that may be None immediately after object creation.

Low
Safely log session_id

Use a safe accessor for session_id and include a fallback value to prevent
misleading or error-prone logs.

tests/get_started.py [37-41]

 driver = webdriver.Remote(
     command_executor=grid_url,
     options=options,
 )
-print(f"Session created: {driver.session_id} ({browser})")
+sid = getattr(driver, "session_id", None)
+print(f"Session created: {sid or 'unknown'} ({browser})")

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Harden logging by safely accessing attributes and providing clear fallbacks in debug prints.

Low
  • More

@VietND96 VietND96 merged commit e74cfea into trunk Oct 17, 2025
29 checks passed
@VietND96 VietND96 deleted the log-id branch October 17, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant