[grid] Fix classpath packaging Redis-backed for SessionQueue

[VietND96]

🔗 Related Issues

💥 What does this PR do?

Two related bugs prevented using a Redis-backed SessionQueue via CLI:

1. --sessionqueue-implementation was not a recognised flag. Passing it produced: Was passed main parameter '--sessionqueue-implementation' but no main parameter was defined in your arg class. The @parameter annotation was simply absent from NewSessionQueueFlags, unlike the equivalent flags in DistributorFlags and SessionMapFlags.
2. RedisBackedNewSessionQueue was not bundled into the sessionqueue server jar. The class is loaded via Class.forName() at runtime, so it must be an explicit Bazel dep of the httpd target even though nothing imports it at compile time. Without it, startup failed with ClassNotFoundException.

🔧 Implementation Notes

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s):
  • What was generated:
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

[qodo-code-review]

PR Summary by Qodo

Fix SessionQueue CLI flag and bundle Redis implementation in httpd jar
🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 10-20 Minutes

Description

• Add missing --sessionqueue-implementation CLI flag mapped to [sessionqueue] implementation.
• Ensure Redis-backed SessionQueue is packaged in the sessionqueue httpd server classpath.
• Add tests covering flag parsing/config output and reflective class loading.

Diagram

graph TD
  A["grid sessionqueue CLI"] --> B["NewSessionQueueFlags"] --> C["Config (AnnotatedConfig)"] --> D["NewSessionQueueOptions"] --> E["NewSessionQueueServer"] --> F["Class.forName() loader"] --> G["RedisBackedNewSessionQueue"]
  H["httpd/BUILD.bazel"] --> I[("sessionqueue httpd jar")]
  I --> E

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use Bazel `runtime_deps` for reflectively-loaded implementations

• ➕ Clarifies intent: dependency is required only at runtime, not for compilation
• ➕ Can reduce compile-time classpath size for httpd target
• ➖ May require Bazel rule adjustments/consistency changes across similar targets
• ➖ Still relies on manual dependency maintenance for reflection

2. Replace reflection with ServiceLoader/AutoService discovery

• ➕ Avoids hard-coded class names and Class.forName() fragility
• ➕ Implementations self-register; fewer packaging footguns
• ➖ Larger refactor affecting configuration semantics and startup behavior
• ➖ Potentially more complex debugging and backward-compat considerations

3. Add a small "linker" class that references known implementations

• ➕ Forces class inclusion without Bazel target edits in some build setups
• ➕ Keeps runtime behavior explicit and testable
• ➖ Hard-codes implementation list in code; needs updates when new impls are added
• ➖ Less clean than declaring correct build dependencies

Recommendation: Keep the PR’s approach: adding the missing flag annotation plus explicit Bazel deps is the smallest, most targeted fix and matches the current reflection-based instantiation model. If packaging issues recur across other reflectively-loaded components, consider standardizing on runtime_deps (or a ServiceLoader-based mechanism) as a follow-up to reduce future classpath regressions.

Files changed (6) +157 / -0

Bug fix (1) +9 / -0

NewSessionQueueFlags.javaAdd '--sessionqueue-implementation' CLI flag mapped to config +9/-0

Add '--sessionqueue-implementation' CLI flag mapped to config

• Adds a JCommander '@Parameter' for '--sessionqueue-implementation' and maps it to '[sessionqueue] implementation' via '@ConfigValue'. This enables selecting non-default SessionQueue implementations (e.g., Redis) via CLI without config-file edits.

java/src/org/openqa/selenium/grid/sessionqueue/config/NewSessionQueueFlags.java

Tests (4) +145 / -0

BUILD.bazelAdd Bazel test suite for sessionqueue config flag tests +15/-0

Add Bazel test suite for sessionqueue config flag tests

• Introduces a 'java_test_suite' to run small tests for sessionqueue config/flags, including JCommander and AssertJ dependencies.

java/test/org/openqa/selenium/grid/sessionqueue/config/BUILD.bazel

NewSessionQueueFlagsTest.javaTest '--sessionqueue-implementation' flag parsing and config output +67/-0

Test '--sessionqueue-implementation' flag parsing and config output

• Adds unit tests ensuring the new CLI flag is accepted by JCommander, populates '[sessionqueue] implementation' in 'AnnotatedConfig', and remains absent when not set.

java/test/org/openqa/selenium/grid/sessionqueue/config/NewSessionQueueFlagsTest.java

BUILD.bazelAdd Bazel test suite validating httpd production classpath +20/-0

Add Bazel test suite validating httpd production classpath

• Adds a small test suite that depends on the httpd target (not redis directly) to verify the production packaging/classpath remains correct over time.

java/test/org/openqa/selenium/grid/sessionqueue/httpd/BUILD.bazel

NewSessionQueueServerTest.javaVerify local and Redis SessionQueue implementations are on httpd classpath +43/-0

Verify local and Redis SessionQueue implementations are on httpd classpath

• Adds reflective 'Class.forName()' tests for both 'LocalNewSessionQueue' and 'RedisBackedNewSessionQueue'. This guards against future Bazel dep changes that would break '--sessionqueue-implementation' at runtime.

java/test/org/openqa/selenium/grid/sessionqueue/httpd/NewSessionQueueServerTest.java

Other (1) +3 / -0

BUILD.bazelInclude Redis SessionQueue in httpd target deps for reflective loading +3/-0

Include Redis SessionQueue in httpd target deps for reflective loading

• Adds an explicit Bazel dependency on '//java/src/org/openqa/selenium/grid/sessionqueue/redis' so 'RedisBackedNewSessionQueue' is packaged into the sessionqueue server jar. Comments document that both local and Redis implementations are loaded via reflection.

java/src/org/openqa/selenium/grid/sessionqueue/httpd/BUILD.bazel

[qodo-code-review]

Code Review by Qodo

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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[diemol]

Why are these comments needed?

[qodo-code-review]

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: CI Success

Failed stage: Verify required jobs succeeded [❌]

Failed test name: ""

Failure summary: The action failed because the workflow step explicitly exited with code 1 after detecting that one or more required jobs failed. - The log shows the step printing One or more required jobs failed and then running exit 1 (lines 27–35). - This appears to be a “CI Success”/gate job that fails the workflow when any upstream required job fails; the actual root failure is in an earlier job not shown in this log excerpt.

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 14: Image: ubuntu-24.04 15: Version: 20260615.205.1 16: Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260615.205/images/ubuntu/Ubuntu2404-Readme.md 17: Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260615.205 18: ##[endgroup] 19: ##[group]GITHUB_TOKEN Permissions 20: Contents: read 21: Metadata: read 22: ##[endgroup] 23: Secret source: Actions 24: Prepare workflow directory 25: Prepare all required actions 26: Complete job name: CI Success 27: ##[group]Run if true; then 28: �[36;1mif true; then�[0m 29: �[36;1m echo "One or more required jobs failed"�[0m 30: �[36;1m exit 1�[0m 31: �[36;1mfi�[0m 32: shell: /usr/bin/bash -e {0} 33: ##[endgroup] 34: One or more required jobs failed 35: ##[error]Process completed with exit code 1. 36: Cleaning up orphan processes