You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
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)
Automated browser version matrix updates with new script
Enhanced CI workflows with parallel builds and streamlined steps
Added version sorting and new browser version support
Consolidated version fetching into unified make target
Diagram Walkthrough
flowchart LR
A["fetch_version.py"] --> B["Browser Matrix YAML"]
C["update_workflow_versions.py"] --> D["Workflow Files"]
B --> E["Makefile Target"]
D --> E
E --> F["CI Workflows"]
F --> G["Automated Updates"]
Loading
File Walkthrough
Relevant files
Enhancement
2 files
fetch_version.py
Enhanced version fetching with sorting and new version support
Casting browser version keys to int when comparing or sorting may raise ValueError if any key is non-numeric (e.g., 'beta', 'lts', or dotted versions). Consider robust parsing or guarding to avoid crashes and ensure stable ordering.
Workflow updater relies on regex line-by-line edits and sets default as a quoted string of a list, which may not remain valid YAML or intended type. Consider YAML-aware editing and ensuring the resulting type matches the workflow input expectations.
defupdate_workflow_file(workflow_file, versions_list):
"""Update the workflow file with new version list for browser-versions.default only."""withopen(workflow_file, 'r') asf:
lines=f.readlines()
updated_lines= []
in_browser_versions=Falsefori, lineinenumerate(lines):
# Check if we're in the browser-versions sectionifre.match(r'^(\s*)browser-versions:\s*$', line):
in_browser_versions=Trueupdated_lines.append(line)
elifin_browser_versionsandre.match(r'^(\s*)default:\s*', line):
# We found the default line within browser-versions sectionindent_match=re.match(r'^(\s*)default:', line)
indent=indent_match.group(1) ifindent_matchelse''# Replace the line with new version listupdated_lines.append(f"{indent}default: '{versions_list}'\n")
in_browser_versions=False# Reset flag after updatingelifin_browser_versionsandre.match(r'^(\s+)(description|required|type):\s*', line):
# Still within browser-versions section, continueupdated_lines.append(line)
elifin_browser_versionsandre.match(r'^(\s*)[a-zA-Z-]+:\s*', line) andnotre.match(r'^(\s+)', line):
# We've moved to another top-level field, reset the flagin_browser_versions=Falseupdated_lines.append(line)
else:
updated_lines.append(line)
withopen(workflow_file, 'w') asf:
f.writelines(updated_lines)
Using --break-system-packages in pip install and adding formatting tools to test requirements may affect CI environments or local systems unexpectedly. Validate reproducibility and isolation (e.g., venv) and that lint tooling in tests requirements is intentional.
install_python_deps:
python3 -m pip install -r tests/requirements.txt --break-system-packages
format_python_scripts: install_python_deps
python3 -m isort tests/ ; \
python3 -m black --line-length=120 --skip-string-normalization tests/
generate_readme_charts:
if [ ! -f $$HOME/go/bin/helm-docs ] ; then \
echo "helm-docs is not installed. Please install it or run 'make setup_dev_env' once." ; \
else \
$$HOME/go/bin/helm-docs --chart-search-root charts/selenium-grid --output-file CONFIGURATION.md --sort-values-order file ; \
fi
update_list_env_vars: install_python_deps
python3 scripts/generate_list_env_vars/extract_env.py
update_selenium_version_matrix: install_python_deps
python3 tests/build-backward-compatible/add_selenium_version.py $(BASE_VERSION)
update_browser_versions_matrix: update_selenium_version_matrix
python3 tests/build-backward-compatible/fetch_firefox_version.py ; \
python3 tests/build-backward-compatible/fetch_version.py ; \
python3 tests/build-backward-compatible/update_workflow_versions.py
The action failed while running the nick-invision/retry@master step executing the command make setup_dev_env. During package installation/setup, the retry action's Node.js process encountered an unhandled exception: Error: kill EPERM (from /home/runner/work/_actions/nick-invision/retry/master/dist/index.js:1931, stack at process.kill (node:internal/process/per_thread:225:13)). - This indicates the retry action attempted to send a signal (kill) to a process but lacked permission or the process was not in a state that allowed signaling, causing the step to crash. - The failure occurred during the setup phase (APT packages being fetched), not due to a specific test failure.
The workflows now fetch browser versions at build time from personal GitHub repos (NDViet/*) and mutate matrices/workflows on-the-fly, introducing a significant supply-chain and reproducibility risk. Source versions from trusted, official endpoints (e.g., OmahaProxy/Chrome, Edge release feeds, Mozilla product APIs) or maintain pinned data in-repo updated via a scheduled PR, with strict schema validation and sanity checks; avoid modifying workflow files during CI runs. This ensures deterministic builds, reduces network fragility, and mitigates the risk of arbitrary upstream changes impacting releases.
Why: The suggestion correctly identifies a critical supply-chain security risk by fetching browser versions from an untrusted personal GitHub repository (NDViet/*) at runtime, which is a major design flaw.
High
General
Validate version number conversion
The function assumes all version keys can be converted to integers, but this could fail if non-numeric versions exist. Add validation to handle potential conversion errors gracefully.
def read_browser_matrix(file_path):
"""Read the browser matrix YAML file and extract browser versions."""
with open(file_path, 'r') as f:
data = yaml.safe_load(f)
chrome_versions = []
firefox_versions = []
edge_versions = []
browsers = data.get('matrix', {}).get('browser', {})
for version, details in browsers.items():
+ try:+ version_int = int(version)+ except ValueError:+ print(f"Skipping non-numeric version: {version}")+ continue+
# Check for Chrome versions (not null or empty)
chrome_version = details.get('CHROME_VERSION')
if chrome_version and chrome_version != 'null' and str(chrome_version).strip():
- chrome_versions.append(int(version))+ chrome_versions.append(version_int)
# Check for Firefox versions (not null or empty)
firefox_version = details.get('FIREFOX_VERSION')
if firefox_version and firefox_version != 'null' and str(firefox_version).strip():
- firefox_versions.append(int(version))+ firefox_versions.append(version_int)
# Check for Edge versions (not null or empty)
edge_version = details.get('EDGE_VERSION')
if edge_version and edge_version != 'null' and str(edge_version).strip():
- edge_versions.append(int(version))+ edge_versions.append(version_int)
Apply / Chat
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies a potential ValueError if version keys in the YAML file are non-numeric, and adding a try-except block makes the script more robust against data format variations.
Low
Handle non-numeric version keys safely
The function assumes all version keys can be converted to integers without validation. This could cause crashes if non-numeric versions exist in the data.
def update_local_yaml(local_data, source_data):
updated = False
local_versions = local_data['matrix']['browser'].keys()
- local_min_version = str(min(int(v) for v in local_versions)) if local_versions else "0"+ try:+ local_min_version = str(min(int(v) for v in local_versions if v.isdigit())) if local_versions else "0"+ except ValueError:+ local_min_version = "0"+
for version, details in source_data['matrix']['browser'].items():
if version in local_data['matrix']['browser']:
original_details = local_data['matrix']['browser'][version]
for key in details:
if key in original_details and '_PACKAGE_' not in key:
original_details[key] = details[key] if details[key] is not None else ""
updated = True
elif '_PACKAGE_' not in key:
original_details[key] = details[key] if details[key] is not None else ""
updated = True
merge_dicts(original_details, details)
else:
- if int(version) > int(local_min_version):- local_data['matrix']['browser'][version] = details- local_data['matrix']['browser'][version]['FIREFOX_PLATFORMS'] = 'linux/amd64,linux/arm64'- updated = True+ try:+ if int(version) > int(local_min_version):+ local_data['matrix']['browser'][version] = details+ local_data['matrix']['browser'][version]['FIREFOX_PLATFORMS'] = 'linux/amd64,linux/arm64'+ updated = True+ except ValueError:+ print(f"Skipping non-numeric version: {version}")+ continue
return updated
Apply / Chat
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly points out that converting version keys to integers without validation can cause a crash, and the proposed change with try-except blocks makes the script more robust.
Low
Add file operation error handling
The function doesn't handle the case where a file might not be found or have permission issues. Add proper error handling to prevent the script from crashing when file operations fail.
def update_workflow_file(workflow_file, versions_list):
"""Update the workflow file with new version list for browser-versions.default only."""
- with open(workflow_file, 'r') as f:- lines = f.readlines()+ try:+ with open(workflow_file, 'r') as f:+ lines = f.readlines()+ except (FileNotFoundError, PermissionError) as e:+ print(f"Error reading {workflow_file}: {e}")+ return
updated_lines = []
in_browser_versions = False
for i, line in enumerate(lines):
# Check if we're in the browser-versions section
if re.match(r'^(\s*)browser-versions:\s*$', line):
in_browser_versions = True
updated_lines.append(line)
elif in_browser_versions and re.match(r'^(\s*)default:\s*', line):
# We found the default line within browser-versions section
indent_match = re.match(r'^(\s*)default:', line)
indent = indent_match.group(1) if indent_match else ''
# Replace the line with new version list
updated_lines.append(f"{indent}default: '{versions_list}'\n")
in_browser_versions = False # Reset flag after updating
elif in_browser_versions and re.match(r'^(\s+)(description|required|type):\s*', line):
# Still within browser-versions section, continue
updated_lines.append(line)
elif in_browser_versions and re.match(r'^(\s*)[a-zA-Z-]+:\s*', line) and not re.match(r'^(\s+)', line):
# We've moved to another top-level field, reset the flag
in_browser_versions = False
updated_lines.append(line)
else:
updated_lines.append(line)
- with open(workflow_file, 'w') as f:- f.writelines(updated_lines)+ try:+ with open(workflow_file, 'w') as f:+ f.writelines(updated_lines)+ except PermissionError as e:+ print(f"Error writing to {workflow_file}: {e}")
Apply / Chat
Suggestion importance[1-10]: 5
__
Why: The suggestion correctly points out missing error handling for file operations, which improves the script's robustness, although the risk of such errors is low in the CI context where it runs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VietND96
force-pushed
the
update-matrix-version
branch
from
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
Motivation and Context
Types of changes
Checklist
PR Type
Enhancement
Description
Automated browser version matrix updates with new script
Enhanced CI workflows with parallel builds and streamlined steps
Added version sorting and new browser version support
Consolidated version fetching into unified make target
Diagram Walkthrough
File Walkthrough
2 files
Enhanced version fetching with sorting and new version supportNew script to automatically update workflow browser versions8 files
Added browser version matrix update stepUpdated default versions and added parallel buildsAdded parallel builds and streamlined version fetchingUpdated default versions and streamlined build processAdded unified browser version update targets and formattingAdded Chrome and Edge version 139 configurationsUpdated browser versions with Firefox 142 and Chrome 140Added Firefox version 142 support1 files
Added documentation for browser version matrix updates1 files
Added Python formatting dependencies