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

Fix #655"Listeners SoftAsserts: Screenshot: null for FAIL test" #659

Merged
merged 1 commit into from Mar 24, 2018

Conversation

Projects
None yet
5 participants
@BorisOsipov
Member

BorisOsipov commented Jan 2, 2018

Proposed changes

Fix #655 issue

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)
Fix #655 - Wrap errors with UIAssertionError
Signed-off-by: Boris Osipov <osipov.boris@gmail.com>
@coveralls

This comment has been minimized.

coveralls commented Jan 2, 2018

Coverage Status

Coverage decreased (-0.04%) to 64.255% when pulling 17e9af0 on 655_SoftAssert_Screenshots into ab55e1d on master.

@codecov-io

This comment has been minimized.

codecov-io commented Jan 2, 2018

Codecov Report

Merging #659 into master will decrease coverage by 0.57%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #659      +/-   ##
============================================
- Coverage     60.43%   59.85%   -0.58%     
- Complexity      770      780      +10     
============================================
  Files           148      148              
  Lines          2750     2805      +55     
  Branches        270      273       +3     
============================================
+ Hits           1662     1679      +17     
- Misses          982     1019      +37     
- Partials        106      107       +1
Impacted Files Coverage Δ Complexity Δ
...ava/com/codeborne/selenide/ElementsCollection.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../codeborne/selenide/impl/SelenideElementProxy.java 86.53% <100%> (ø) 26 <0> (ø) ⬇️
...deborne/selenide/impl/SelenideElementIterator.java 91.66% <0%> (-8.34%) 10% <0%> (+5%)
...e/selenide/impl/WebDriverThreadLocalContainer.java 79.71% <0%> (-0.73%) 30% <0%> (ø)
...com/codeborne/selenide/impl/CollectionElement.java 100% <0%> (ø) 12% <0%> (+5%) ⬆️

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 ab55e1d...17e9af0. Read the comment docs.

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Jan 3, 2018

It again complains about WebDriverThreadLocalContainer.java coverage decrease but it was not affected in the changes..

@BorisOsipov BorisOsipov closed this Jan 3, 2018

@BorisOsipov BorisOsipov reopened this Jan 3, 2018

@vinogradoff

This comment has been minimized.

Member

vinogradoff commented Jan 3, 2018

@coveralls

This comment has been minimized.

coveralls commented Jan 3, 2018

Coverage Status

Coverage increased (+0.008%) to 64.299% when pulling 17e9af0 on 655_SoftAssert_Screenshots into ab55e1d on master.

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Jan 3, 2018

Yes it is. But some times it reports unexpected coverage changes here https://github.com/codeborne/selenide/blob/0f2e4502f85b3b46184373536309b2da0db956cb/src/main/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainer.java#L41

and we can't fix it :)
Also I checked Codeconv documentation. It cannot exclude specific lines from coverage reports.

@BorisOsipov BorisOsipov added the ready label Jan 4, 2018

@rosolko

This comment has been minimized.

Collaborator

rosolko commented Mar 23, 2018

@BorisOsipov Can you please try to replace foreach with stream if reduce this if:

ALL_WEB_DRIVERS_THREADS.stream()
  .filter(thread -> !thread.isAlive())
  .forEach(thread -> {
    log.info("Thread " + thread.getId() + " is dead. Let's close webdriver " + THREAD_WEB_DRIVER.get(thread.getId()));
    closeWebDriver(thread);
  });

Possible it can help avoid false codecov trigger.

@rosolko rosolko added the question label Mar 23, 2018

@BorisOsipov

This comment has been minimized.

Member

BorisOsipov commented Mar 24, 2018

@rosolko Let's make a separate issue about code coverage flashing and test your proposal?
I don't think such changes directly reference to this PR changes.

@rosolko rosolko merged commit 125a9c0 into master Mar 24, 2018

2 of 4 checks passed

codecov/patch 50% of diff hit (target 60.43%)
Details
codecov/project 59.85% (-0.58%) compared to ab55e1d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.008%) to 64.299%
Details

@rosolko rosolko deleted the 655_SoftAssert_Screenshots branch Mar 24, 2018

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