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

Remove /^I wait for the page to load$/ #136

Closed
judithroth opened this issue Apr 2, 2020 · 2 comments
Closed

Remove /^I wait for the page to load$/ #136

judithroth opened this issue Apr 2, 2020 · 2 comments

Comments

@judithroth
Copy link
Contributor

Using steps like this is a bad pattern. It would be better to check the page for the expected result (e.g. check that something is now visible) (see this). We should try not to encourage bad patterns.
The step also relies on jQuery, which is unexpected and could fail.

👍 vote for removing
👎 vote for keeping
@makmic
Copy link
Member

makmic commented Apr 6, 2020

I clearly agree that this step should be avoided whenever possible. Nevertheless it will complicate the update to Spreewald 3 for many of our projects. Maybe they just have to copy and adjust the step for their need (e.g. w/o jQuery).

@judithroth
Copy link
Contributor Author

To avoid unnecessary version bumps (and confusion) we should approach all "remove xy" issues at once. Also, before removing, we want to release a version x.99 which prints deprecation warnings for everything that is removed in version x+1.

nhasselmeyer added a commit that referenced this issue Oct 1, 2020
@nhasselmeyer
Remove deprecated steps
322fc7c 
- The following steps have been removed:
  - `/^the file "([^"]*)" was attached(?: as (?:([^"]*)\/)?([^"]*))? to the ([^"]*) above(?: at "([^"]*)")?$/` (see [#106](#106))
  - `/^the "([^"]*)" field should have no error$/` (see [#134](#134))
  - `/^I should get a text response$/` (see [#135](#135))
  - `/^I wait for the page to load$/` (see [#136](#136))
  - `debugger` (was an alias for `console`)
  - `/^(an|no) e?mail should have been sent((?: |and|with|from "[^"]+"|bcc "[^"]+"|cc "[^"]+"|to "[^"]+"|the subject "[^"]+"|the body "[^"]+"|the attachments "[^"]+")+)$/`,
  `/^that e?mail should( not)? have the following lines in the body$/` and
  `/^that e?mail should have the following (?:|content in the )body:$/` (see [#132](#132 (comment)))
- The step `/^(the tag )?"([^\"]+)" should( not)? be visible$/` lost its `not` modifier (Use `Then (the tag )?"..." should be hidden` or `Then I should not see "..."`)
- The file `lib/spreewald/timecop_steps.rb` was removed (was an alias for `lib/spreewald/time_steps.rb`).
nhasselmeyer added a commit that referenced this issue Oct 1, 2020
@nhasselmeyer
Remove deprecated steps
5a4c827 
- The following steps have been removed:
  - `/^the file "([^"]*)" was attached(?: as (?:([^"]*)\/)?([^"]*))? to the ([^"]*) above(?: at "([^"]*)")?$/` (see [#106](#106))
  - `/^the "([^"]*)" field should have no error$/` (see [#134](#134))
  - `/^I should get a text response$/` (see [#135](#135))
  - `/^I wait for the page to load$/` (see [#136](#136))
  - `debugger` (was an alias for `console`)
  - `/^(an|no) e?mail should have been sent((?: |and|with|from "[^"]+"|bcc "[^"]+"|cc "[^"]+"|to "[^"]+"|the subject "[^"]+"|the body "[^"]+"|the attachments "[^"]+")+)$/`,
  `/^that e?mail should( not)? have the following lines in the body$/` and
  `/^that e?mail should have the following (?:|content in the )body:$/` (see [#132](#132 (comment)))
- The step `/^(the tag )?"([^\"]+)" should( not)? be visible$/` lost its `not` modifier (Use `Then (the tag )?"..." should be hidden` or `Then I should not see "..."`)
- The file `lib/spreewald/timecop_steps.rb` was removed (was an alias for `lib/spreewald/time_steps.rb`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@judithroth @makmic @nhasselmeyer