Skip to content
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

[py]: #11093 - The SessionId shouldn't be added to params themself bu… #11121

Merged
merged 8 commits into from
Oct 29, 2022

Conversation

tvataire
Copy link
Contributor

@tvataire tvataire commented Oct 13, 2022

…t to a copy of them.

Description

The Webdriver.execute() method shouldn't add the SessionId to its params themself but to a copy of them.
This avoid the SessionId to be preserved in the PrintOptions instance.

Motivation and Context

If the session id is stored inside the PrintOptions instance, a selenium.common.exceptions.InvalidSessionIdException is raised when the PrintOptions instance is reused in a subsequent session.

Types of changes

  • [x ] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [x ] I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@titusfortner
Copy link
Member

ah, nice solution. Can you add a unit test?

@tvataire
Copy link
Contributor Author

Hi
An unit test is ready but I couldn't find the way to run the tests.
When I run a bazel command, nothing happens.
I tried these commands :
bazel build //py:selenium
bazel test //py:test-firefox
A bazel process is started, but it never ends and nothing is displayed on stdout.
Can somebody help me to fix this.

@tvataire
Copy link
Contributor Author

Ok, my install of bazel was broken.
Seems to work properly now...

@titusfortner titusfortner requested a review from symonk October 26, 2022 16:41
@titusfortner titusfortner added this to the 4.6 milestone Oct 28, 2022
@codecov-commenter
Copy link

Codecov Report

Base: 52.67% // Head: 52.67% // No change to project coverage 👍

Coverage data is based on head (7312569) compared to base (aa23847).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #11121   +/-   ##
=======================================
  Coverage   52.67%   52.67%           
=======================================
  Files          82       82           
  Lines        5542     5542           
  Branches      198      198           
=======================================
  Hits         2919     2919           
  Misses       2425     2425           
  Partials      198      198           
Impacted Files Coverage Δ
py/selenium/webdriver/remote/webdriver.py 41.65% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@titusfortner titusfortner merged commit e2542bb into SeleniumHQ:trunk Oct 29, 2022
@titusfortner
Copy link
Member

thanks!

joshmgrant pushed a commit to joshmgrant/selenium that referenced this pull request Oct 29, 2022
…hemself bu… (SeleniumHQ#11121)

* [py]: SeleniumHQ#11093 - The SessionId shouldn't be added to params themself but to a copy of them.

* [py] Add an unit test to check the session id is not preserved in print options after a page is printed.

* [py] Update the test_session_id_is_not_preserved_after_page_is_printed test to comply with coding conventions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants