Skip to content
This repository was archived by the owner on Jul 20, 2025. It is now read-only.

Conversation

@kasatani
Copy link

In current implementation of ghostdriver there is no way to load cookies when starting a session. Passing --cookies-file to phantomjs doesn't work because ghostdriver's session.js creates a new cookie jar without any argument, which doesn't allow us to load or save cookies.

I have added "phantomjs.cookies.path" capability that lets you specify the path for the cookie jar.

@jesg jesg self-requested a review July 21, 2017 01:53
Copy link
Collaborator

@jesg jesg left a comment

Choose a reason for hiding this comment

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

thanks for working on this. looks like a good addition to ghostdriver.

i need two changes to accept this feature.

  1. the default case should be backwards compatible

  2. basic unit test to verify this actually sets cookies

_currentWindowHandle = null,
_cookieJar = require('cookiejar').create(),
_cookieJar,
_cookiePath = null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

the default should be an empty string to be consistent with the phantomjs source. though i'd prefer to be conservative and pass no arguments when this option is not used.

src/session.js Outdated
_cookiePath = desiredCapabilities['phantomjs.cookies.path'];
_negotiatedCapabilities['phantomjs.cookies.path'] = _cookiePath;
}
_cookieJar = require('cookiejar').create(_cookiePath);
Copy link
Collaborator

Choose a reason for hiding this comment

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

the default case should be the same. require('cookiejar').create()

@kasatani
Copy link
Author

kasatani commented Oct 6, 2017

Thanks for the feedback. I've fixed the behavior when the path is not set, and added the test case.

@jesg
Copy link
Collaborator

jesg commented Oct 23, 2017

@kasatani the test ghostdriver.CookieStoreTest#shouldLoadCookies does not pass when i run it.

to run the test:

cd <ghostdriver project root>/test/java
./gradlew test -Dtest.single=CookieStoreTest  --debug

you may need to modify phantomjs_exec_path in <project root>/test/config.ini to phantomjs on your system.

@kasatani
Copy link
Author

CookieStoreTest passes in my environment. Which version of phantomjs should I use for the testing? I'm using the released phantomjs-2.1.1 on macOS Sierra.

@jesg
Copy link
Collaborator

jesg commented Nov 3, 2017

Which version of phantomjs should I use for the testing?

i normally use a patched version of 2.1.1 but i'm also seeing the error in the official 2.1.1 release. i run the tests on ubuntu 17.

i should be able to take a look on a mac sometime next week.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants