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

Add '?' symbol to filename pattern to avoid url query string parameters. #735

Merged
merged 2 commits into from May 10, 2018

Conversation

Projects
None yet
5 participants
@rosolko
Collaborator

rosolko commented May 10, 2018

Fix #734 issue.

Proposed changes

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that 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)

@rosolko rosolko added the bug 🐛 label May 10, 2018

@rosolko rosolko added this to the 4.11.5 milestone May 10, 2018

@rosolko rosolko self-assigned this May 10, 2018

@rosolko rosolko requested review from asolntsev and BorisOsipov May 10, 2018

@asolntsev

@rosolko There could be some test for this change...

@rosolko

This comment has been minimized.

Collaborator

rosolko commented May 10, 2018

@asolntsev Not so easy, as long as this value going from http headers.
So simple adding query string parameters as described in bug for element href attribute ins't working.

@codecov-io

This comment has been minimized.

codecov-io commented May 10, 2018

Codecov Report

Merging #735 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #735      +/-   ##
============================================
- Coverage     63.61%   63.58%   -0.04%     
  Complexity      878      878              
============================================
  Files           154      154              
  Lines          3029     3029              
  Branches        298      298              
============================================
- Hits           1927     1926       -1     
- Misses         1003     1004       +1     
  Partials         99       99
Impacted Files Coverage Δ Complexity Δ
...n/java/com/codeborne/selenide/impl/HttpHelper.java 84.61% <100%> (ø) 6 <0> (ø) ⬇️
...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 e216b36...4f7dc78. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented May 10, 2018

Coverage Status

Coverage decreased (-0.03%) to 66.854% when pulling 4f7dc78 on rosolko:download_filename_pattern into e216b36 on codeborne:master.

@rosolko

This comment has been minimized.

Collaborator

rosolko commented May 10, 2018

@BorisOsipov Good catch, thanks!

@rosolko rosolko merged commit 7159996 into selenide:master May 10, 2018

3 of 4 checks passed

coverage/coveralls Coverage decreased (-0.03%) to 66.854%
Details
codecov/patch 100% of diff hit (target 63.61%)
Details
codecov/project Absolute coverage decreased by -0.03% but relative coverage increased by +36.38% compared to e216b36
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rosolko rosolko deleted the rosolko:download_filename_pattern branch May 10, 2018

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