ExcludedArguments functionality for Chromium-based drivers #144
Conversation
…+semver: feature closes #143 Also remove "--remote-allow-origins=*" workaround Add --disable-search-engine-choice-screen to settings.json
WalkthroughThe recent changes enhance the configuration capabilities of Chromium-based drivers by introducing an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChromeSettings
participant DriverSettings
participant EdgeSettings
User->>ChromeSettings: Initialize ChromeOptions
ChromeSettings->>DriverSettings: Get Excluded Arguments
DriverSettings-->>ChromeSettings: Return Excluded Arguments
ChromeSettings->>ChromeOptions: Set Excluded Arguments
User->>EdgeSettings: Initialize EdgeOptions
EdgeSettings->>DriverSettings: Get Excluded Arguments
DriverSettings-->>EdgeSettings: Return Excluded Arguments
EdgeSettings->>EdgeOptions: Set Excluded Arguments
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
mialeska
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/main/java/aquality/selenium/configuration/driversettings/ChromeSettings.java (2 hunks)
- src/main/java/aquality/selenium/configuration/driversettings/DriverSettings.java (6 hunks)
- src/main/java/aquality/selenium/configuration/driversettings/EdgeSettings.java (1 hunks)
- src/main/resources/settings.json (4 hunks)
- src/test/resources/settings.json (4 hunks)
Additional comments not posted (17)
src/main/java/aquality/selenium/configuration/driversettings/EdgeSettings.java (1)
23-23: LGTM! The addition enhances flexibility.The call to
setExcludedArguments(edgeOptions)integrates well into thegetDriverOptionsmethod, enhancing flexibility in configuring the browser's startup behavior.src/main/java/aquality/selenium/configuration/driversettings/ChromeSettings.java (2)
23-23: LGTM! The addition enhances flexibility.The call to
setExcludedArguments(chromeOptions)integrates well into thegetDriverOptionsmethod, enhancing flexibility in configuring the browser's startup behavior.
Line range hint
19-22:
LGTM! The removal of the workaround is appropriate.The removal of the workaround for the
--remote-allow-origins=*flag is appropriate, indicating that it is no longer necessary or the underlying issue has been resolved.src/main/resources/settings.json (5)
23-24: LGTM! Enhances configurability.The addition of the
--disable-search-engine-choice-screenargument and theexcludedArgumentsfield enhances the configurability of the Chrome browser.
40-41: LGTM! Enhances configurability.The addition of the
excludedArgumentsfield enhances the configurability of the Edge browser.
Line range hint
70-71:
LGTM! Enhances configurability.The addition of the
excludedArgumentsfield enhances the configurability of the Firefox browser.
84-85: LGTM! Enhances configurability.The addition of the
excludedArgumentsfield enhances the configurability of the Opera browser.
99-100: LGTM! Enhances configurability.The addition of the
excludedArgumentsfield enhances the configurability of the Yandex browser.src/test/resources/settings.json (5)
22-23: LGTM: AddedstartArgumentsandexcludedArgumentsfor Chrome.The addition of
--disable-search-engine-choice-screentostartArgumentsandenable-automationtoexcludedArgumentsaligns with the PR objectives and enhances automation control.
39-40: LGTM: AddedexcludedArgumentsfor Edge.The addition of
enable-automationtoexcludedArgumentsaligns with the PR objectives and enhances automation control.
Line range hint
54-55:
LGTM: AddedexcludedArgumentsfor Firefox.The addition of
enable-automationtoexcludedArgumentsaligns with the PR objectives and enhances automation control.
83-84: LGTM: AddedexcludedArgumentsfor Opera.The addition of
enable-automationtoexcludedArgumentsaligns with the PR objectives and enhances automation control.
98-99: LGTM: AddedexcludedArgumentsfor Yandex.The addition of
enable-automationtoexcludedArgumentsaligns with the PR objectives and enhances automation control.src/main/java/aquality/selenium/configuration/driversettings/DriverSettings.java (4)
29-29: LGTM: AddedexcludedArgumentsfield.The addition of the
excludedArgumentsfield is necessary for managing excluded arguments and aligns with the PR objectives.
113-113: LGTM: ModifiedlogCollectionmethod visibility.The change from
private finaltoprivateallows for potential overrides, enhancing flexibility.
141-143: LGTM: AddedsetExcludedArgumentsmethod.The method correctly sets the excluded arguments in
ChromiumOptionsusing thesetExperimentalOptionmethod.
169-169: LGTM: AddedEXCLUDED_ARGSentry toCapabilityTypeenum.The addition of the
EXCLUDED_ARGSentry is necessary for managing excluded arguments and aligns with the PR objectives.
src/main/java/aquality/selenium/configuration/driversettings/DriverSettings.java
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/main/java/aquality/selenium/configuration/driversettings/DriverSettings.java (6 hunks)
- src/main/resources/localization/be.json (1 hunks)
- src/main/resources/localization/en.json (1 hunks)
- src/main/resources/localization/pl.json (1 hunks)
- src/main/resources/localization/ru.json (1 hunks)
- src/main/resources/localization/uk.json (1 hunks)
Files skipped from review due to trivial changes (1)
- src/main/resources/localization/pl.json
Additional comments not posted (8)
src/main/java/aquality/selenium/configuration/driversettings/DriverSettings.java (4)
Line range hint
113-118:
LGTM!The
logCollectionmethod is correctly implemented.
141-143: LGTM!The
setExcludedArgumentsmethod is correctly implemented.
169-171: LGTM!The
CapabilityTypeenum is correctly updated with the new entry forEXCLUDED_ARGS.
86-101: Fix logging issue ingetExcludedArgumentsmethod.The method correctly retrieves and logs excluded arguments, but it incorrectly logs
startArgumentsinstead ofexcludedArguments.- logCollection("loc.browser.excludedArguments", startArguments); + logCollection("loc.browser.excludedArguments", excludedArguments);Likely invalid or redundant comment.
src/main/resources/localization/en.json (1)
3-3: LGTM!The new localization entry for
loc.browser.excludedArgumentsis correctly added.src/main/resources/localization/be.json (1)
3-3: LGTM!The new localization entry for
loc.browser.excludedArgumentsis correctly added.src/main/resources/localization/uk.json (1)
3-3: New localization entry approved.The new entry "loc.browser.excludedArguments" is correctly translated and consistent with the existing entries.
src/main/resources/localization/ru.json (1)
3-3: New localization entry approved.The new entry "loc.browser.excludedArguments" is correctly translated and consistent with the existing entries.
…+semver: feature
closes #143
Also remove "--remote-allow-origins=*" workaround
Add --disable-search-engine-choice-screen to settings.json