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

introduce setValueChangeEvent option #718

Merged
merged 4 commits into from Apr 25, 2018

Conversation

Projects
None yet
6 participants
@MikeShysh
Contributor

MikeShysh commented Apr 23, 2018

Proposed changes

New option "setValueChangeEvent". It's needed to avoid firing change event.

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 Apr 23, 2018

Codecov Report

Merging #718 into master will increase coverage by <.01%.
The diff coverage is 80%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #718      +/-   ##
===========================================
+ Coverage     63.49%   63.5%   +<.01%     
- Complexity      862     864       +2     
===========================================
  Files           154     154              
  Lines          2978    2981       +3     
  Branches        288     290       +2     
===========================================
+ Hits           1891    1893       +2     
- Misses          988     989       +1     
  Partials         99      99
Impacted Files Coverage Δ Complexity Δ
...ain/java/com/codeborne/selenide/Configuration.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...java/com/codeborne/selenide/commands/SetValue.java 96.96% <100%> (+0.09%) 13 <0> (+1) ⬆️
...n/java/com/codeborne/selenide/commands/Append.java 100% <100%> (ø) 3 <0> (+1) ⬆️

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 688db05...e8b5dbb. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented Apr 23, 2018

Coverage Status

Coverage decreased (-0.0002%) to 66.823% when pulling e8b5dbb on MikeShysh:setValueChangeEventOption into 688db05 on codeborne:master.

@rosolko

This comment has been minimized.

Collaborator

rosolko commented Apr 23, 2018

@MikeShysh Can you please provide some tests for this ability?

@rosolko

This comment has been minimized.

Collaborator

rosolko commented Apr 24, 2018

@MikeShysh, some tests failing, please take a look at them:

> Task :htmlunit FAILED
430 tests completed, 2 failed, 50 skipped
integration.PageWithoutJQuery > setValueDoesNotTriggerOnChangeEvent FAILED
    Element should have text '_' {#username-mirror}
    Element: '<h3 id="username-mirror">(1)</h3>'
    Screenshot: 
    Timeout: 4 s.
        at com.codeborne.selenide.impl.WebElementSource.checkCondition(WebElementSource.java:66)
        at com.codeborne.selenide.commands.Should.should(Should.java:35)
        at com.codeborne.selenide.commands.Should.execute(Should.java:29)
        at com.codeborne.selenide.commands.Should.execute(Should.java:12)
        at com.codeborne.selenide.commands.Commands.execute(Commands.java:144)
        at com.codeborne.selenide.impl.SelenideElementProxy.dispatchAndRetry(SelenideElementProxy.java:89)
        at com.codeborne.selenide.impl.SelenideElementProxy.invoke(SelenideElementProxy.java:65)
        at com.sun.proxy.$Proxy27.shouldHave(Unknown Source)
        at integration.PageWithoutJQuery.setValueDoesNotTriggerOnChangeEvent(PageWithoutJQuery.java:32)
integration.PageWithoutJQuery > setValueTriggersOnChangeEvent FAILED
    Element should have text 'john' {#username-mirror}
    Element: '<h3 id="username-mirror">(1)</h3>'
    Screenshot: 
    Timeout: 4 s.
        at com.codeborne.selenide.impl.WebElementSource.checkCondition(WebElementSource.java:66)
        at com.codeborne.selenide.commands.Should.should(Should.java:35)
        at com.codeborne.selenide.commands.Should.execute(Should.java:29)
        at com.codeborne.selenide.commands.Should.execute(Should.java:12)
        at com.codeborne.selenide.commands.Commands.execute(Commands.java:144)
        at com.codeborne.selenide.impl.SelenideElementProxy.dispatchAndRetry(SelenideElementProxy.java:89)
        at com.codeborne.selenide.impl.SelenideElementProxy.invoke(SelenideElementProxy.java:65)
        at com.sun.proxy.$Proxy27.shouldHave(Unknown Source)
        at integration.PageWithoutJQuery.setValueTriggersOnChangeEvent(PageWithoutJQuery.java:19)
@MikeShysh

This comment has been minimized.

Contributor

MikeShysh commented Apr 24, 2018

Fixed. Thanks, @BorisOsipov and @rosolko for help

@@ -32,7 +32,7 @@ task safari(type: Test) {
task htmlunit(type: Test) {
systemProperties['selenide.browser'] = 'htmlunit'
include 'integration/**/*'
exclude 'com/codeborne/selenide/**/*'
exclude 'com/codeborne/selenide/**/*', 'integration/*PageWithoutJQuery*'

This comment has been minimized.

@BorisOsipov

BorisOsipov Apr 24, 2018

Member

@MikeShysh please, revert this and use assumeFalse(isHtmlUnit()); in test you would like to skip for htmlunit

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Apr 24, 2018

@MikeShysh One tiny issue but after that this is good to go.

@rosolko rosolko added this to the 4.11.2 milestone Apr 24, 2018

@MikeShysh

This comment has been minimized.

Contributor

MikeShysh commented Apr 24, 2018

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Apr 24, 2018

@MikeShysh awesome 👍 Thank you for handle it.

@rosolko rosolko merged commit 7715163 into selenide:master Apr 25, 2018

3 of 4 checks passed

coverage/coveralls Coverage decreased (-0.0002%) to 66.823%
Details
codecov/patch 80% of diff hit (target 63.49%)
Details
codecov/project 63.5% (+<.01%) compared to 688db05
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@asolntsev asolntsev self-assigned this Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment