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)
flowchart LR
A["Java Test Suite"] --> B["Bootstrap Script"]
B --> C["Docker Container"]
C --> D["Selenium Grid"]
D --> E["Browser Tests"]
F["CI Workflows"] --> B
G["Makefile"] --> B
The script uses exit_code=$? to capture the exit code from gradlew but doesn't handle the case where the variable might not be set if the script exits before line 34. The cleanup function references this variable which could be undefined.
The package declaration dev.selenium doesn't match the file structure. The test file is located in the default package but declares a specific package, which could cause compilation issues.
Using basic assert statements instead of JUnit assertions reduces test readability and debugging capabilities. JUnit assertions provide better error messages and are the standard practice.
The script doesn't properly handle the exit code from the cleanup function. The exit_code variable is set after ./gradlew clean test but the cleanup function references it before it's defined, which could cause undefined behavior.
Why: The suggestion correctly identifies that the exit_code variable is uninitialized if the script fails before the tests run, causing the script to incorrectly exit with status 0.
Medium
General
Replace assert with JUnit assertions
Using assert statements for test assertions is problematic because they can be disabled with -da JVM flag and don't provide meaningful error messages. Use JUnit assertions instead for proper test failure reporting.
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that using JUnit assertions is better practice than native assert statements for tests, as they provide more informative failure messages and are not disabled by default.
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.
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
Tests
Description
Add Java-based Selenium tests with BiDi support
Integrate Java tests into CI/CD workflows
Create test infrastructure for all browsers
Enable modern RemoteWebDriver patterns
Changes diagram
Changes walkthrough 📝
6 files
Create Java test bootstrap scriptAdd Java tests to Chrome workflowAdd Java tests to Edge workflowAdd Java tests to Firefox workflowAdd Java test targets for browsersImplement Java Selenium tests with BiDi1 files
Add Java test documentation3 files
Configure Gradle wrapper propertiesAdd Gradle wrapper scriptAdd Gradle wrapper batch script