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.

Sign up for GitHub

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

Jump to bottom

Add 'expiry' to 'add cookie' keyword and add 'get cookie' keyword. #930

Merged
merged 9 commits into from
Oct 5, 2017
Merged

Add 'expiry' to 'add cookie' keyword and add 'get cookie' keyword. #930

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a242eae
Add 'expiry' to 'add cookie' keyword and add get cookie expiry keyword.
wappowers Sep 28, 2017
3938b59
Datetime format is now used for expiry setting. Get cookie keyword ad…
wappowers Sep 28, 2017
1e9b12a
Using Collections library for dictionary reliant acceptance test.
wappowers Sep 28, 2017
7f556b6
Merge branch 'master' of https://github.com/robotframework/SeleniumLi…
wappowers Sep 29, 2017
52f4db4
Refactor to have 'get cookie' return an extended variable rather than…
wappowers Sep 29, 2017
56730c3
Updated to use Robot Framework DateTime library, and PEP8 changes.
wappowers Oct 2, 2017
63f0209
Acceptance tests pass regardless of timezone.
wappowers Oct 2, 2017
6b57846
Added name to CHANGES.rst file.
wappowers Oct 3, 2017
dfa3fab
Added full_info option to CookieInformation class, and some code revi…
wappowers Oct 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions src/SeleniumLibrary/keywords/cookie.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

from SeleniumLibrary.base import LibraryComponent, keyword
from SeleniumLibrary.utils import is_truthy
from robot.libraries.DateTime import convert_date


class CookieKeywords(LibraryComponent):
Expand Down Expand Up @@ -53,11 +54,28 @@ def get_cookie_value(self, name):
raise ValueError("Cookie with name %s not found." % name)

@keyword
def add_cookie(self, name, value, path=None, domain=None, secure=None):
def get_cookie(self, name):
"""Returns the cookie found with `name` in extended variable format.

If no cookie is found with `name`, this keyword fails.
"""
cookie = self.browser.get_cookie(name)
if cookie:
cookie_information = CookieInformation(
cookie['name'],cookie['value'], cookie['domain'],
cookie['path'],cookie['expiry'], cookie['httpOnly'],
cookie['secure'])
return cookie_information
raise ValueError("Cookie with name %s not found." % name)

@keyword
def add_cookie(self, name, value, path=None, domain=None, secure=None,
expiry=None):
"""Adds a cookie to your current session.

"name" and "value" are required, "path", "domain" and "secure" are
optional
"name" and "value" are required, "path", "domain", "secure" and
"expiry" are optional. Expiry supports the same formats as
the DateTime library.
"""
new_cookie = {'name': name, 'value': value}
if is_truthy(path):
Expand All @@ -67,4 +85,21 @@ def add_cookie(self, name, value, path=None, domain=None, secure=None):
# Secure should be True or False
if is_truthy(secure):
new_cookie['secure'] = secure
if is_truthy(expiry):
expiry_datetime = int(convert_date(expiry, result_format='epoch'))
new_cookie['expiry'] = expiry_datetime
self.browser.add_cookie(new_cookie)


class CookieInformation(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better ro write CookieInformation class like this:

class CookieInformation(object):

    def __init__(self, **cookie_dict):
        for key in cookie_dict:
            setattr(self, key, cookie_dict[key])

And write line 64 and 65 like this:
return CookieInformation(**cookie)

Because selenium cookie is a dictionary and at least in Windows get_cookie does not return expiry if it not defined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, such a generic __init__ would be even better implemented like this:

def __init__(self, data):
    self.__dict__.update(data)

I'm don't think that is a good idea in this context, though:

  • Explicit is better than implicit. We are saving few lines of code but hiding information. Being able to use this object you needed to know what info the Selenium method returned.
  • Making it sure we always return same info regardless what browsers return is important.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then something like this could work better:

class CookieInformation(object):

    def __init__(self, cookie):
        self.domain = cookie['domain'] if 'domain' in cookie else None
        self.expiry = cookie['expiry'] if 'expiry' in cookie else None
        self.httpOnly = cookie['httpOnly'] if 'httpOnly' in cookie else None
        self.name = cookie['name'] if 'name' in cookie else None
        self.path = cookie['path'] if 'path' in cookie else None
        self.secure = cookie['secure'] if 'secure' in cookie else None
        self.value = cookie['value'] if 'value' in cookie else None

    @property
    def full_info(self):
        return self.__dict__

def __init__(self, name, value, domain, path, expiry, http, secure):
self.name = name
self.value = value
self.domain = domain
self.path = path
self.expiry = expiry
self.http = http
self.secure = secure

def __str__(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __str__ method is not needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If actually is. Robot automatically logs return values from kws and without this method the message is useless. Should be discussed what info to show, though. Could only show name and value and keep the message short or return all the info. I'd probably keep __str__ simple but could add a separate property showing everything that could be used like

 Log    ${cookie.full_info}

if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not know that it is needed. Most of the time, I am interested about the name and value, because for me they are the most important information which I need. I think we can leave it as is and Pekka idea about full_info could be useful.

return '{}={}'.format(self.name, self.value)
10 changes: 9 additions & 1 deletion test/acceptance/keywords/cookies.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Suite Teardown Delete All Cookies
Test Setup Add Cookies
Resource ../resource.robot


*** Test Cases ***
Get Cookies
[Documentation] Get Cookies
Expand Down Expand Up @@ -48,9 +49,16 @@ Get Cookies When There Are None
${cookies}= Get Cookies
Should Be Equal ${cookies} ${EMPTY}

Get Cookie Expiry Set By Selenium
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FF indeed seems to have a problem with cookies, please add a tag to the test Known_Issue_Firefox and CI will automatically mark this test as noncritical for Firefox.

Did you happen to try with other browsers, like IE, Edge or Safari, that do they suffer from the same problem?

[Documentation] Get Cookie Value Set By Selenium
[Tags] Known Issue Firefox
${cookie}= Get Cookie another
${date}= Convert Date 2027-09-28 16:21:35 epoch
should be equal as integers ${cookie.expiry} ${date}

*** Keyword ***
Add Cookies
[Documentation] Add Cookies
Delete All Cookies
Add Cookie test seleniumlibrary
Add Cookie another value
Add Cookie another value expiry=2027-09-28 16:21:35
1 change: 1 addition & 0 deletions test/acceptance/resource.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Library SeleniumLibrary run_on_failure=Nothing implicit_wait=0
Library Collections
Library OperatingSystem
Library DateTime

*** Variable ***
${SERVER}= localhost:7000
Expand Down