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 steps to switch between sessions #142

Closed
wants to merge 2 commits into from
Closed

Add steps to switch between sessions #142

wants to merge 2 commits into from

Conversation

nhasselmeyer
Copy link
Collaborator

Closes #66

@makmic makmic self-requested a review August 5, 2020 13:40
Copy link
Member

@makmic makmic left a comment

Choose a reason for hiding this comment

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

I added a few suggestions, thanks for the good work!
When you are done, please test your steps in the real world project we talked about.

CHANGELOG.md Outdated
@@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file.

This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## Unreleased
- Add a step modifier to control different sessions: `... in the browser session "..."`. (see issue [#66](https://github.com/makandra/spreewald/issues/66))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Add a step modifier to control different sessions: `... in the browser session "..."`. (see issue [#66](https://github.com/makandra/spreewald/issues/66))
- Add a step modifier to control different Capybara sessions: `... in the browser session "..."`. (see issue [#66](https://github.com/makandra/spreewald/issues/66))

end.overridable

# nodoc
When /^(.*) in the browser session "([^"]+):$/ do |nested_step, session_name, table_or_string|
Copy link
Member

Choose a reason for hiding this comment

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

I'm glad you thought about the interoperability with table steps!
Please add a test for this step as well.

Feature: Browser session steps

Background: Access some content in the main session
Given I go to "/static_pages/tab_1"
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 introduce a Background if it's only used by a single Scenario

Given I go to "/static_pages/tab_1"
Then I should see "First browser tab"

Scenario: Browser sessions do not interfere with each other
Copy link
Member

Choose a reason for hiding this comment

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

Please add new minimal static pages for this test. It's irritating to read about browser tabs when they do not matter at all

Given I go to "/static_pages/tab_1"
Then I should see "First browser tab"

Scenario: Browser sessions do not interfere with each other
Copy link
Member

Choose a reason for hiding this comment

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

When you are checking multiple assertions in a single scenario, express the assertion as a comment.

In this case, I would group it sth. like this:

# Different pages can be accessed on different browser sessions
..
 I should see "second session" in the browser session "Second browser session"
...

# The default session is not affected by actions in other sessions
Then I should not see "Second browser session"
  And I should not see "Third browser session"

# Other sessions are not affected by the default session
...


Then I should see "Second browser tab" in the browser session "second"
And I should see "Third browser tab" in the browser session "third"
But I should not see "First browser tab" in the browser session "second"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
But I should not see "First browser tab" in the browser session "second"
But I should not see "First browser tab" in the browser session "second"

@@ -0,0 +1 @@
../../shared/features/shared/browser_tab_steps.feature
Copy link
Member

Choose a reason for hiding this comment

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

I thought all shared features are already symlinked through their hierarchy, isn't that so?

@@ -286,6 +286,18 @@ the step definitions.
This step is deprecated and will be removed from spreewald. If you still want to use it, copy the code to your project's own steps.


### session_steps.rb
Copy link
Member

Choose a reason for hiding this comment

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

👍 Good description! Thanks for moving things to a separate file.

@makmic makmic closed this Aug 12, 2020
@nhasselmeyer nhasselmeyer deleted the nh/session-steps branch August 12, 2020 16:18
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.

Step to switch Capybara sessions
2 participants