-
Notifications
You must be signed in to change notification settings - Fork 35
Update to Selenium 4.34.0 +semver:feature #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Sonatype deploy: publish with central-publishing-maven-plugin, use maven versions plugin
WalkthroughThis update upgrades Selenium DevTools protocol imports from version 137 to 138 across source and test files, adjusts Maven POM dependencies and plugins to newer versions, and modernizes the Maven Central publishing workflow. The Maven project version and repository configuration are also revised, with some plugins replaced or newly configured. Additionally, Azure pipeline tasks for SonarCloud integration and Maven usage were enhanced, and Chrome driver settings were added or updated in test resources. Minor test code improvements and UI navigation logic cleanup were also applied. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Maven Central
Developer->>GitHub Actions: Push code / trigger release workflow
GitHub Actions->>GitHub Actions: Set project version via Maven (versions:set)
GitHub Actions->>GitHub Actions: Deploy to Maven Central (skip tests)
GitHub Actions->>Maven Central: Publish artifacts
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/release-maven-central.yml (1)
23-27
:setup-java
step is given variable names instead of secret values
actions/setup-java
expects the actual credentials, not the names of variables. With the current config, Maven settings.xml will contain the literalsMVN_CENTRAL_USERNAME
/MVN_CENTRAL_PASSWORD
, causing 401s on deploy.Fix:
- server-id: central - server-username: MVN_CENTRAL_USERNAME - server-password: MVN_CENTRAL_PASSWORD - gpg-passphrase: MVN_GPG_PASSPHRASE + server-id: central + server-username: ${{ secrets.OSSRH_USERNAME }} + server-password: ${{ secrets.OSSRH_TOKEN }} + gpg-passphrase: ${{ secrets.GPG_PASSPHRASE }}(or export the env vars before this step).
Without this adjustment the workflow will fail at the very first Maven invocation.
🧹 Nitpick comments (1)
pom.xml (1)
235-238
:versions-maven-plugin
is added but unusedGreat to have the plugin pinned; however, no execution or profile invokes it.
If you intend to rely on it from the workflow only, that’s fine; otherwise consider adding a goal (e.g.versions:display-dependency-updates
) to a maintenance profile.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/release-maven-central.yml
(2 hunks)pom.xml
(10 hunks)src/main/java/aquality/selenium/browser/devtools/DevToolsHandling.java
(1 hunks)src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java
(1 hunks)src/main/java/aquality/selenium/browser/devtools/JavaScriptHandling.java
(1 hunks)src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java
(8 hunks)src/test/java/tests/usecases/devtools/DeviceEmulationTest.java
(1 hunks)src/test/java/tests/usecases/devtools/NetworkInterceptionTests.java
(3 hunks)src/test/java/tests/usecases/devtools/NetworkSpeedEmulationTest.java
(1 hunks)src/test/java/tests/usecases/devtools/OverrideUserAgentTest.java
(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
src/test/java/tests/usecases/devtools/OverrideUserAgentTest.java (2)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java (2)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
src/main/java/aquality/selenium/browser/devtools/DevToolsHandling.java (2)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
src/main/java/aquality/selenium/browser/devtools/JavaScriptHandling.java (2)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
src/test/java/tests/usecases/devtools/DeviceEmulationTest.java (2)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java (2)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
src/test/java/tests/usecases/devtools/NetworkSpeedEmulationTest.java (1)
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
🔇 Additional comments (18)
src/main/java/aquality/selenium/browser/devtools/JavaScriptHandling.java (1)
15-17
: Version bump looks safe – verify compile & runtime classpath alignmentSwitching DevTools imports from
v137
tov138
is a clean mechanical upgrade. Please double-check:
selenium-devtools-v138
is present in the POM and on the runtime classpath.- Dependent APIs (
Page.removeScriptToEvaluateOnNewDocument
,Runtime.*
) kept compatible signatures between 137 → 138 – a full compile will surface any mismatch.- No modules still reference v137; mixing versions typically results in
ClassCastException
s.Assuming the rest of the PR aligns, this change looks good. 👍
pom.xml (1)
224-233
:central-publishing-maven-plugin
introduced, but remember it disables snapshot deploymentSwitching from
nexus-staging-maven-plugin
tocentral-publishing-maven-plugin
will successfully release to Central, but SNAPSHOT pushes now have no target repository because<distributionManagement>
was removed.
Verify that you don’t rely onmvn deploy -DskipTests
for nightly/snapshot builds, or add an alternate snapshot repository in the company settings if required..github/workflows/release-maven-central.yml (1)
57-63
: Nice split betweenversions:set
anddeploy
Separating version bump from deployment keeps logs clear and avoids re-running the entire lifecycle if versioning fails. Good job.
src/main/java/aquality/selenium/browser/devtools/DevToolsHandling.java (1)
12-13
: Please manually verify Selenium dependency versions
I wasn’t able to locate anyselenium-java
orselenium-devtools
coordinates in the build scripts via automated search. To ensure your v138 imports compile correctly, confirm that:
- Your project depends on Selenium ≥ 4.34.0 (first release that includes v138 stubs).
- You’ve declared the matching DevTools artifact (
selenium-devtools-v138
or the umbrellaselenium-devtools
).For Maven, check your
pom.xml
(s):<dependency> <groupId>org.seleniumhq.selenium</groupId> <artifactId>selenium-java</artifactId> <version>4.34.0</version> </dependency> <dependency> <groupId>org.seleniumhq.selenium</groupId> <artifactId>selenium-devtools-v138</artifactId> <version>4.34.0</version> </dependency>For Gradle:
implementation 'org.seleniumhq.selenium:selenium-java:4.34.0' implementation 'org.seleniumhq.selenium:selenium-devtools-v138:4.34.0'Once you’ve confirmed or updated these, the migration to v138 will be fully complete.
src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java (1)
5-8
: Confirm API compatibility after bumping to v138
RGBA
,Emulation
,MediaFeature
, andScreenOrientation
moved to the v138 namespace, but their method signatures occasionally change between protocol revisions.
Compile-time will catch mismatches, but CI doesn’t always run locally—please runmvn test-compile
(or the Gradle equivalent) before merging.src/test/java/tests/usecases/devtools/NetworkSpeedEmulationTest.java (1)
5-5
: Import version updated – looks goodThe test keeps the logic intact while aligning with DevTools v138.
src/test/java/tests/usecases/devtools/OverrideUserAgentTest.java (1)
11-11
: Import bump acknowledgedNo behavioural change; compilation only.
src/test/java/tests/usecases/devtools/DeviceEmulationTest.java (1)
6-7
: Import bump acknowledgedDevice-emulation tests stay unchanged except for the namespace switch.
src/test/java/tests/usecases/devtools/NetworkInterceptionTests.java (4)
8-8
: LGTM: Import updated for modernized API usage.The import of
Contents
class aligns with the updated Selenium API and supports the more concise content setting approach used in the test methods.
22-22
: LGTM: Static import enhances readability.The static import for
utf8String
method improves code readability and follows modern Java practices.
90-90
: LGTM: Modernized content setting approach.The change from
new ByteArrayInputStream(somePhrase.getBytes())
toContents.utf8String(somePhrase)
is a good modernization that:
- Eliminates manual byte array handling
- Uses the appropriate utility method from Selenium's API
- Maintains the same functionality with cleaner code
109-109
: LGTM: Consistent modernization of content setting.This change mirrors the improvement in the request handler test, maintaining consistency across the test suite while using the modernized Selenium API.
src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java (6)
11-11
: LGTM: Import updated for DevTools v138.The import statement correctly references the v138 network model classes, consistent with the broader upgrade.
25-25
: LGTM: Static import updated consistently.The static import correctly references the v138 Network class, maintaining consistency with the model import.
29-29
: LGTM: Javadoc reference updated.The Javadoc correctly references the v138 Network class, ensuring documentation accuracy.
202-202
: LGTM: Response body retrieval updated to v138 namespace.The fully qualified class name correctly references the v138 Network class for the
getResponseBody()
method call, ensuring consistency with the import changes.
182-182
: Verify DevTools Request API method names
Please confirm that in your Selenium/CDP dependency (v138+), theRequest
class definesgetPostDataEntries()
and thatgetPostData()
has been removed or renamed.• Location:
src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java:182
• Snippet:request.getPostDataEntries().orElse(null)This change assumes an API update in CDP v138—make sure your project’s DevTools version matches.
132-132
: Verify Network.enable() signature
The calls toenable(Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())
now include four parameters, suggesting the v138 API’sNetwork.enable
method has expanded its signature. Please confirm that inorg.openqa.selenium.devtools.v138.network.Network
, theenable
command indeed accepts fourOptional
arguments.Applicable locations:
- src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java:132
- src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java:142
- src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java:325
- src/main/java/aquality/selenium/browser/devtools/NetworkHandling.java:341
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
azure-pipelines.yml
(1 hunks)src/test/resources/settings.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/resources/settings.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#148
File: src/test/java/tests/usecases/devtools/DeviceEmulationTest.java:6-7
Timestamp: 2024-11-29T14:35:07.825Z
Learning: In this project, the use of custom DevTools Protocol version (v131) in test files is intentional, even if it differs from the version used in the main source code.
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-java#150
File: src/main/java/aquality/selenium/browser/devtools/EmulationHandling.java:121-121
Timestamp: 2025-05-28T19:55:33.768Z
Learning: The DevTools v137 `Emulation.setDeviceMetricsOverride` method has 14 parameters, not 13 as some documentation might suggest. Recent versions of the Selenium DevTools API have added additional parameters to this method.
|
Sonatype deploy: publish with central-publishing-maven-plugin, use maven versions plugin