migrate video_nodeQuery.sh to python

[KyriosGN0]

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

migrate the video_nodeQuery.sh to python
partially closes #2650

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.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Inconsistency The record_video logic may not match the original shell script behavior. The current implementation treats any value other than string "false" as "true", which could lead to unexpected behavior if the value is null, undefined, or a different type. if isinstance(record_video, str) and record_video.lower() == "false": record_video = "false" else: record_video = "true" Redundant Assignment The conditional assignment for test_name has a redundant case. Line 58 assigns test_name to itself, which is unnecessary and could be removed. elif test_name and test_name != "null": test_name = test_name

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Handle boolean values correctly Suggestion Impact: The commit implemented exactly the suggested change, adding a condition to check if record_video is False (boolean) in addition to checking if it's the string 'false' code diff: - if isinstance(record_video, str) and record_video.lower() == "false": + if (isinstance(record_video, str) and record_video.lower() == "false") or record_video is False: The current logic doesn't properly handle boolean values in JSON. If record_video is a boolean False (not string "false"), it will be incorrectly set to "true". Add a check for boolean type to correctly process both string and boolean values. Video/video_nodeQuery.py [48-52] # Check if enabling to record video -if isinstance(record_video, str) and record_video.lower() == "false": +if (isinstance(record_video, str) and record_video.lower() == "false") or record_video is False: record_video = "false" else: record_video = "true" [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that boolean False values from JSON aren't handled properly, which could cause incorrect behavior when record_video is a boolean instead of a string. This is a legitimate bug fix.	Medium
	✅ Improve regex pattern handling Suggestion Impact: The suggestion was directly implemented in the commit. The code was changed to handle multiple POSIX character classes (alnum, alpha, digit, space) exactly as suggested, replacing the previous implementation that only handled [:alnum:]. code diff: # Convert trim pattern to regex # Handle character classes like [:alnum:] - if "[:alnum:]" in trim_pattern: - # Create regex pattern for alphanumeric characters plus other allowed chars - allowed_chars = trim_pattern.replace("[:alnum:]", "a-zA-Z0-9") - pattern = f"[^{allowed_chars}]" - else: - # Direct character set - pattern = f"[^{re.escape(trim_pattern)}]" + posix_classes = { + "[:alnum:]": "a-zA-Z0-9", + "[:alpha:]": "a-zA-Z", + "[:digit:]": "0-9", + "[:space:]": " \t\n\r\f\v" + } + + allowed_chars = trim_pattern + for posix_class, replacement in posix_classes.items(): + if posix_class in allowed_chars: + allowed_chars = allowed_chars.replace(posix_class, replacement) + + pattern = f"[^{re.escape(allowed_chars)}]" The regex pattern handling is incomplete for POSIX character classes. The current implementation only handles [:alnum:] but ignores other potential classes like [:digit:] or [:alpha:]. Implement a more comprehensive POSIX class conversion. Video/video_nodeQuery.py [93-101] # Convert trim pattern to regex # Handle character classes like [:alnum:] -if "[:alnum:]" in trim_pattern: - # Create regex pattern for alphanumeric characters plus other allowed chars - allowed_chars = trim_pattern.replace("[:alnum:]", "a-zA-Z0-9") - pattern = f"[^{allowed_chars}]" -else: - # Direct character set - pattern = f"[^{re.escape(trim_pattern)}]" +posix_classes = { + "[:alnum:]": "a-zA-Z0-9", + "[:alpha:]": "a-zA-Z", + "[:digit:]": "0-9", + "[:space:]": " \t\n\r\f\v" +} +allowed_chars = trim_pattern +for posix_class, replacement in posix_classes.items(): + if posix_class in allowed_chars: + allowed_chars = allowed_chars.replace(posix_class, replacement) + +pattern = f"[^{re.escape(allowed_chars)}]" + [Suggestion processed] Suggestion importance[1-10]: 4 __ Why: While the suggestion adds support for more POSIX character classes, the current implementation may be sufficient for the specific use case. This is an enhancement rather than fixing a critical issue.	Low
General	Remove redundant code The condition test_name = test_name is redundant and doesn't change anything. This makes the code confusing and less maintainable. Simplify the logic by removing the redundant assignment. Video/video_nodeQuery.py [54-60] # Check if video file name is set via capabilities if video_name and video_name != "null": test_name = video_name -elif test_name and test_name != "null": - test_name = test_name -else: +elif not (test_name and test_name != "null"): test_name = "" Apply / Chat Suggestion importance[1-10]: 2 __ Why: The suggestion correctly identifies redundant code (test_name = test_name), but the proposed fix changes the logic incorrectly by negating the condition, which would alter the intended behavior.	Low
Update

[KyriosGN0]

@VietND96 is there some doc on how to replicate CI failures locally?

[Copilot]

Pull Request Overview

This PR replaces the existing shell-based video query logic with a Python implementation and updates the main video script to call the new Python script.

• Remove legacy video_nodeQuery.sh and add video_nodeQuery.py
• Update video.sh to invoke Python instead of Bash
• Reimplement normalization and capability parsing in Python

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
Video/video_nodeQuery.sh	Removed legacy Bash implementation
Video/video_nodeQuery.py	Added new Python script for session video configuration
Video/video.sh	Updated call site to use the Python script

Comments suppressed due to low confidence (2)

Video/video_nodeQuery.py:10

• There are no tests covering main() or normalize_filename(). Consider adding unit tests for argument parsing, JSON decoding, and filename normalization to ensure correctness.

def main() -> None:

Video/video_nodeQuery.py:29

• [nitpick] The variable name video_file_name_trim might be more descriptive if it reflected that it's a regex (e.g., file_name_trim_regex).

video_file_name_trim = os.environ.get("SE_VIDEO_FILE_NAME_TRIM_REGEX", "[:alnum:]-_")