[java] Merge capabilities of slot with the new session request capabilities#11369
[java] Merge capabilities of slot with the new session request capabilities#11369pujagani merged 11 commits intoSeleniumHQ:trunkfrom
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
|
We do something similar on the |
|
Thank you for helping take a look. The LocalNode logic, is for sending the result back to the user. The PR does something similar but to send to the driver. Mainly, if the sessionSlot is configured with custom capabilities, the PR merges that with incoming session capabilities and sends it to the driver factory. My thought process was more around the session slot being the owner of the stereotype, the logic should sit there. LocaNode is more of a meditator and is aware of the session slots. What do you think? |
bdbe3a0 to
a51bda4
Compare
…ity to user sent capabilities
|
@diemol After giving your comments a thought, it made sense to move the logic to SessionCapabilitiesMutator since that is where we make some checks on the capabilities and add some capabilities as required. Additionally, as you correctly pointed out, I think it makes sense to give the user-sent capabilities priority over the stereotype capability to avoid any breaking changes. I have updated the changes accordingly and reverted the previous ones. Will appreciate your review. Thank you so much. |
diemol
left a comment
There was a problem hiding this comment.
Thank you, I left a few comments.
java/src/org/openqa/selenium/grid/node/config/SessionCapabilitiesMutator.java
Show resolved
Hide resolved
java/src/org/openqa/selenium/grid/node/config/SessionCapabilitiesMutator.java
Outdated
Show resolved
Hide resolved
java/src/org/openqa/selenium/grid/node/config/SessionCapabilitiesMutator.java
Show resolved
Hide resolved
diemol
left a comment
There was a problem hiding this comment.
Added missing comment for the Chromium case.
java/src/org/openqa/selenium/grid/node/config/SessionCapabilitiesMutator.java
Outdated
Show resolved
Hide resolved
…ity to user sent capabilities
…selenium into merge-node-config-caps
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #11369 +/- ##
=======================================
Coverage 54.65% 54.65%
=======================================
Files 85 85
Lines 5643 5643
Branches 244 244
=======================================
Hits 3084 3084
Misses 2315 2315
Partials 244 244 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Merge capabilities of slot with the new session request capabilities
Motivation and Context
Node config allows the Grid user to set driver configuration.
Example:
java -jar selenium-server-4.6.0.jar standalone --port 50859 --detect-drivers false --driver-configuration display-name="Chrome Custom" stereotype='{"browserName":"chrome","goog:chromeOptions":{"args":[\"incognito\",\"window-size=500,500\"]},"pageLoadStrategy":"normal"}'The configuration is applied to the session slots.
This intends to provide the configuration that all matching incoming session requests should ideally consider and pass it along to the respective web driver.
Currently, the new session request capabilities are passed as it is without considering if the session slot has additional capabilities that need to be considered.
The changes help fix this by giving merging the capabilities of the slot and the incoming session request. The capabilities of the incoming session request override the capabilities of the slot case of custom driver-configuration.
Types of changes
Checklist