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

Add automatic soft isolation to Session docs #52

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

j4m3s
Copy link

@j4m3s j4m3s commented Feb 26, 2015

Includes reference to Selenium limitation which results in cookies bleeding between multi-domain scenarios discussed in #51

Includes reference to Selenium limitation which results in cookies bleeding between multi-domain scenarios.
@stof
Copy link
Member

stof commented Feb 27, 2015

Your rST syntax is invalid (see the travis build)

Mink provides two very useful methods to isolate tests, which can be used
in your test's ``teardown`` methods:
for acceptance tests. But a very important part in testing is isolation.
Mink tries to automatically "soft" isolate Scenarios from each other without
Copy link
Member

Choose a reason for hiding this comment

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

this documentation should not talk about scenarios. Scenarios are a Behat concept, not a Mink one. Mink is not tied to Behat

Remove invalid indentation and reference to Scenarios
@j4m3s
Copy link
Author

j4m3s commented Mar 3, 2015

Thanks for flagging those - I didn't get a notification about the failed build, but have checked this one manually and the syntax seems ok now. I assumed the syntax was ok when the github preview worked. Dangerous to assume! I did try pasting the whole file into the rST validator at https://www.tele3.cz/jbar/rest/rest.html to make sure I was now correct, but it came up with a pile of validation errors in parts of the file I hadn't changed so I just gave it a try against Travis (twice). Apologies for the multiple attempts.

I've removed the reference to Scenarios - I'm afraid my use of Mink through Behat alone led me to another assumption... The doc now refers to tests instead.

@stof
Copy link
Member

stof commented Mar 3, 2015

@j4m3s you don't receive emails when the PR status is updated. This only appears on the PR after it is created or updated (the build takes a few minutes)

@@ -183,10 +183,17 @@ Resetting the Session
---------------------

The primary aim for Mink is to provide a single consistent web browsing API
for acceptance tests. But a very important part in testing is isolation.
for acceptance tests. But a very important part in testing is isolation.
Mink tries to automatically "soft" isolate tests from each other without
Copy link
Member

Choose a reason for hiding this comment

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

Mink does not do anything automatically. Both reset and stop must be called explicitly

@j4m3s
Copy link
Author

j4m3s commented Mar 3, 2015

@stof I'm confused here. In issue #51 you said these were the docs to update. Are you suggesting that the answer to my question 'am I right in thinking that scenarios are "soft isolated" by default' should not be documented here after all?

The whole thread was born from cookies bleeding between scenarios so if Mink knows nothing about scenarios and does nothing automatically isolate it seems I'm in the wrong place entirely.

@aik099
Copy link
Member

aik099 commented Mar 3, 2015

Are you suggesting that the answer to my question 'am I right in thinking that scenarios are "soft isolated" by default' should not be documented here after all?

You're correct @j4m3s , because scenarios are Behat/MinkExtension terminology. Mink docs shouldn't mention anything of Behat.

@j4m3s
Copy link
Author

j4m3s commented Mar 3, 2015

Understood, thanks - let me revise the pull request.

Is there somewhere that I should document the cookie-bleed risk? MinkExtension maybe? I have to admit I struggle somewhat with the Behat 3 readthedocs.

@j4m3s
Copy link
Author

j4m3s commented Mar 3, 2015

I've made something of a pig's ear of this PR - multiple commits while we ground out the right text, plus several fixing the rst syntax. Once you're happy with the content let me know if you'd like me to raise a new PR with a single commit.

@aik099
Copy link
Member

aik099 commented Mar 3, 2015

Looks good, @j4m3s . @stof , any corrections before merge?

@@ -183,10 +183,10 @@ Resetting the Session
---------------------

The primary aim for Mink is to provide a single consistent web browsing API
for acceptance tests. But a very important part in testing is isolation.
for acceptance tests. But a very important part in testing is isolation.
Copy link
Member

Choose a reason for hiding this comment

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

Please don't add trailing whitespaces

Copy link
Member

Choose a reason for hiding this comment

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

Better yet enable automatic trailing space removal in your editor.

@phil-davis
Copy link
Contributor

FYI I wrote some words for MinkExtension here:
Behat/MinkExtension#292
that documents how @insulated works with Behat-MinkExtension on its way to calling Mink stuff. (obviously not to go in Mink doc, since that is a MinkExtension feature)

It would be nice to have some of the words in this PR in the Mink doc, as it explains a bit more about what happens for reset vs stop in Mink.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants