Skip to content

[py] Add unit tests for selenium.webdriver.remote.client_config#17390

Open
EternalRights wants to merge 4 commits intoSeleniumHQ:trunkfrom
EternalRights:add-client-config-unit-tests
Open

[py] Add unit tests for selenium.webdriver.remote.client_config#17390
EternalRights wants to merge 4 commits intoSeleniumHQ:trunkfrom
EternalRights:add-client-config-unit-tests

Conversation

@EternalRights
Copy link
Copy Markdown

Description

Adds unit test coverage for selenium.webdriver.remote.client_config, which currently has no tests.

Covers:

  • AuthType enum values
  • _ClientConfigDescriptor get/set behavior
  • ClientConfig.__init__ defaults and explicit values
  • timeout default (socket fallback) and explicit values
  • ca_certs default, from env, and explicit path
  • reset_timeout()
  • get_auth_header() for BASIC / BEARER / X-API-Key (including missing credential edge cases)
  • get_proxy_url() for DIRECT, SYSTEM (env var handling), and MANUAL proxy types

Type of change

  • New tests (missing test coverage)

Verification

All 27 tests pass locally:

27 passed in 0.22s

covers AuthType enum, _ClientConfigDescriptor, ClientConfig
init defaults, timeout, ca_certs, reset_timeout, get_auth_header
for all AuthType variants, and get_proxy_url for DIRECT/SYSTEM/MANUAL
proxy types including env var handling

Signed-off-by: EternalRights <3147268827@qq.com>
@selenium-ci selenium-ci added the C-py Python Bindings label Apr 25, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add unit tests for selenium.webdriver.remote.client_config

🧪 Tests

Grey Divider

Walkthroughs

Description
• Adds comprehensive unit test coverage for selenium.webdriver.remote.client_config
• Tests AuthType enum values and _ClientConfigDescriptor behavior
• Tests ClientConfig initialization with defaults and explicit values
• Tests timeout, ca_certs, reset_timeout, authentication headers, and proxy URL resolution

Grey Divider

File Changes

1. py/test/unit/selenium/webdriver/remote/client_config_tests.py 🧪 Tests +267/-0

Comprehensive unit tests for client_config module

• New test file with 27 unit tests covering AuthType enum, _ClientConfigDescriptor, and
 ClientConfig class
• Tests initialization defaults and explicit parameter values
• Tests timeout handling with socket fallback and ca_certs from environment or explicit paths
• Tests reset_timeout() method and get_auth_header() for BASIC, BEARER, and X-API-Key
 authentication types
• Tests get_proxy_url() for DIRECT, SYSTEM (with environment variable handling), and MANUAL proxy
 types

py/test/unit/selenium/webdriver/remote/client_config_tests.py


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 25, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Unused pytest import🐞 Bug ⚙ Maintainability
Description
client_config_tests.py imports pytest but never uses it, which will fail Ruff (Pyflakes)
unused-import checks and break CI/linting for this PR.
Code

py/test/unit/selenium/webdriver/remote/client_config_tests.py[22]

+import pytest
Evidence
The new test file contains import pytest with no references to pytest anywhere else in the
module. The repo enables Ruff's Pyflakes rules (including F401 unused imports), so this will be
reported as a lint error.

py/test/unit/selenium/webdriver/remote/client_config_tests.py[18-26]
py/pyproject.toml[140-166]

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

### Issue description
`py/test/unit/selenium/webdriver/remote/client_config_tests.py` imports `pytest` but doesn't use it. Ruff linting (Pyflakes F401) will flag this as an unused import and fail CI.

### Issue Context
The tests use pytest fixtures (e.g., `monkeypatch`) but do not call the `pytest` module directly, so the import is unnecessary.

### Fix Focus Areas
- py/test/unit/selenium/webdriver/remote/client_config_tests.py[18-26]

### Suggested change
Remove `import pytest` (or start using `pytest` explicitly if needed).

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



Advisory comments

2. Proxy test comments restate what📘 Rule violation ⚙ Maintainability
Description
Several added comments in test_proxy_url_system_no_env describe obvious mechanics instead of
explaining the rationale for the setup. This adds noise and reduces maintainability contrary to the
comment guidance.
Code

py/test/unit/selenium/webdriver/remote/client_config_tests.py[R210-220]

+def test_proxy_url_system_no_env():
+    """SYSTEM proxy without env vars set should return None or http_proxy/https_proxy."""
+    config = _make_config(remote_server_addr="http://example.com:4444")
+    # SYSTEM with no env vars → os.environ.get returns None → get_proxy_url returns None
+    with patch.dict(os.environ, {}, clear=False):
+        # Make sure no proxy env vars are set
+        for key in ["http_proxy", "HTTP_PROXY", "https_proxy", "HTTPS_PROXY", "no_proxy", "NO_PROXY"]:
+            os.environ.pop(key, None)
+        url = config.get_proxy_url()
+        # With no env vars, should be None
+        assert url is None
Evidence
PR Compliance ID 9 requires comments to explain rationale ('why') rather than restating code
behavior ('what'); the added comments in this test describe the env-var manipulation and expected
return value, which is already clear from the code and assertions.

AGENTS.md
py/test/unit/selenium/webdriver/remote/client_config_tests.py[210-220]

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

## Issue description
New comments in the unit test restate the mechanics of the code ("what") rather than the rationale ("why"), creating noisy documentation.

## Issue Context
The test already clearly shows the environment manipulation and expected behavior via `patch.dict`, `os.environ.pop(...)`, and `assert url is None`.

## Fix Focus Areas
- py/test/unit/selenium/webdriver/remote/client_config_tests.py[210-220]

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


Grey Divider

Qodo Logo

Comment thread py/test/unit/selenium/webdriver/remote/client_config_tests.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants