Skip to content

[bazel+js]: Avoid shadowing directory with test name#16784

Merged
shs96c merged 1 commit intoSeleniumHQ:trunkfrom
shs96c:js-firefox-tests
Dec 23, 2025
Merged

[bazel+js]: Avoid shadowing directory with test name#16784
shs96c merged 1 commit intoSeleniumHQ:trunkfrom
shs96c:js-firefox-tests

Conversation

@shs96c
Copy link
Member

@shs96c shs96c commented Dec 23, 2025 •
edited by qodo-code-review bot
Loading

User description

The test target name test collides with the source directory test/ containing the test HTML files. When Bazel builds the runfiles tree:

  • The default test target (named test) places its binary at _main/javascript/atoms/test (a file)
  • The test HTML files should be at _main/javascript/atoms/test/*.html (in a directory)
  • Since the binary occupies the test path as a file, the test/ directory with HTML files cannot exist in the runfiles Files.find() returns 0 test files because the test directory doesn't exist (it's a file instead)

Renaming the tests avoids this problem.

Insert facepalm -->here<--

PR Type

Bug fix

Description

  • Rename test targets from test to closure-test to avoid shadowing

  • Prevents file/directory collision in Bazel runfiles tree

  • Fixes test discovery issue where HTML test files were inaccessible

  • Updates documentation and examples with new target names

Diagram Walkthrough

flowchart LR
  A["Test target named 'test'"] -->|"Collides with"| B["test/ directory"]
  B -->|"Causes"| C["Runfiles tree conflict"]
  C -->|"Results in"| D["0 test files found"]
  E["Rename to 'closure-test'"] -->|"Resolves"| C
  E -->|"Enables"| F["Proper test discovery"]
Loading

File Walkthrough
Relevant files
Bug fix
BUILD.bazel
Rename atoms test target to closure-test                                 

javascript/atoms/BUILD.bazel

  • Renamed closure_test_suite target from test to closure-test
  • Resolves file/directory shadowing issue in Bazel runfiles
 +1/-1     
BUILD.bazel
Rename chrome-driver test target to closure-test                 

javascript/chrome-driver/BUILD.bazel

  • Renamed closure_test_suite target from test to closure-test
  • Prevents naming collision with test directory
 +1/-1     
BUILD.bazel
Rename webdriver test target to closure-test                         

javascript/webdriver/BUILD.bazel

  • Renamed closure_test_suite target from test to closure-test
  • Aligns with other JavaScript test target naming changes
 +1/-1     
Documentation
README.md
Update documentation with new test target name                     

javascript/atoms/README.md

  • Updated debug server command to use closure-test_debug_server
  • Updated test execution examples to reference closure-test target
  • Updated test filtering example with new target name
 +3/-3     

@shs96c
[bazel+js]: Avoid shadowing directory with test name
8ac91be 
The test target name `test` collides with the source directory
`test/` containing the test HTML files. When Bazel builds the
runfiles tree:

* The default `test` target (named `test`) places its binary at
  `_main/javascript/atoms/test` (a file)
* The test HTML files should be at
  `_main/javascript/atoms/test/*.html` (in a directory)
* Since the binary occupies the test path as a file, the `test/`
  directory with HTML files cannot exist in the runfiles
  `Files.find()` returns 0 test files because the test directory
   doesn't exist (it's a file instead)

Renaming the tests avoids this problem.

Insert facepalm -->here<--
@selenium-ci selenium-ci added B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations labels Dec 23, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Dec 23, 2025

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
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Dec 23, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@shs96c shs96c merged commit 15947c3 into SeleniumHQ:trunk Dec 23, 2025
15 checks passed
@shs96c shs96c deleted the js-firefox-tests branch December 23, 2025 11:27
@titusfortner
Copy link
Member

titusfortner commented Dec 23, 2025

hah, I fixed this in another PR. Your fix is better. 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shs96c @titusfortner @selenium-ci