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

Chromeoptions prefs #692

Merged
merged 6 commits into from Mar 13, 2018

Conversation

Projects
None yet
6 participants
@sirdir
Contributor

sirdir commented Feb 7, 2018

Proposed changes

Need use "prefs" via setExperimentalOption() for ChromeOptions
https://sites.google.com/a/chromium.org/chromedriver/capabilities#TOC-Set-a-Chrome-preference

Propose to set "prefs" like currently "args" working -Dchromeoptions.args=
-Dchromeoptions.prefs=
e.g. -Dchromeoptions.prefs=profile.block_third_party_cookies=false,profile.avatar_index=26

Checklist

  • Checkstyle and unit tests pass locally with my changes by running gradle check chrome htmlunit command
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
@codecov-io

This comment has been minimized.

codecov-io commented Feb 7, 2018

Codecov Report

Merging #692 into master will increase coverage by 0.22%.
The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #692      +/-   ##
============================================
+ Coverage     61.23%   61.46%   +0.22%     
- Complexity      797      809      +12     
============================================
  Files           152      152              
  Lines          2817     2839      +22     
  Branches        271      275       +4     
============================================
+ Hits           1725     1745      +20     
- Misses          986      987       +1     
- Partials        106      107       +1
Impacted Files Coverage Δ Complexity Δ
...ain/java/com/codeborne/selenide/Configuration.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...eborne/selenide/webdriver/ChromeDriverFactory.java 86% <88.88%> (+7.42%) 20 <10> (+12) ⬆️
...e/selenide/impl/WebDriverThreadLocalContainer.java 80.88% <0%> (-0.74%) 30% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca2b0bd...f3a762e. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented Feb 7, 2018

Coverage Status

Coverage increased (+0.2%) to 65.234% when pulling f3a762e on sirdir:chromeoptions_prefs into ca2b0bd on codeborne:master.

@BorisOsipov

This comment has been minimized.

@BorisOsipov BorisOsipov self-requested a review Feb 7, 2018

@BorisOsipov

Need tests

@sirdir

This comment has been minimized.

Contributor

sirdir commented Feb 7, 2018

@BorisOsipov I will try.

}
}
}
return currentChromeOptions;
}
private Object parseString(String value) {

This comment has been minimized.

@vinogradoff

vinogradoff Feb 7, 2018

Member

lol, I needed 10 minutes to understand how it works.
So prefs maps has Int/Boolean/Strings as value and you map "true/false" to boolean, numbers to int, and the rest stays string. Could you try to comment this (if I am right) in this method? :) It will help future readers to understand the magic, it looked strange to me at first....

This comment has been minimized.

@sirdir
typo in ChromeDriverFactoryTest
add 3 test for parsing prefs
add description for map value parsing
@sirdir

This comment has been minimized.

Contributor

sirdir commented Feb 7, 2018

please check
@BorisOsipov tests done
@vinogradoff description added

@sirdir

This comment has been minimized.

Contributor

sirdir commented Feb 17, 2018

Fix\add tests for chrome preferences transfer. Simplify code.
Signed-off-by: Boris Osipov <osipov.boris@gmail.com>
@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Feb 17, 2018

Thank you @sirdir

I reformated\refactored your code a bit.
We will merge it for the nearest releases.

@BorisOsipov BorisOsipov added the ready label Feb 17, 2018

@asolntsev asolntsev merged commit 9c33c7b into selenide:master Mar 13, 2018

4 checks passed

codecov/patch 88.88% of diff hit (target 61.23%)
Details
codecov/project 61.46% (+0.22%) compared to ca2b0bd
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 65.234%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment