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

Method $ should NOT automatically open a browser #809

Closed
asolntsev opened this Issue Sep 23, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@asolntsev
Copy link
Contributor

asolntsev commented Sep 23, 2018

When using Selenide, a normal flow is to first open a browser, and then acting with elements. Like this:

open(url);
$(...).do(...);

The problem

If one forgers to call open and calls $, Selenide still opens a browser. It's kind of unexpected behaviour. It can cause mysterious bugs.

Solution

I suggest to change method $, so that it throws an exception if a browser is not open yet.

@asolntsev asolntsev added this to the 5.0.0 milestone Sep 23, 2018

@asolntsev asolntsev self-assigned this Sep 23, 2018

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Sep 23, 2018

Implemented in branch selenide-driver

@BorisOsipov

This comment has been minimized.

Copy link
Member

BorisOsipov commented Sep 23, 2018

@asolntsev just to be sure. For $$().someaction() the same?

@asolntsev

This comment has been minimized.

Copy link
Contributor

asolntsev commented Sep 24, 2018

@BorisOsipov Yes, sure. Both $ and $$ should throw an exception if browser is not opened.

@rosolko

This comment has been minimized.

Copy link
Collaborator

rosolko commented Sep 24, 2018

It's a good point! Awesome 🎉

@rosolko rosolko modified the milestones: 5.0.0-rc.1, 5.0.0-rc.2 Oct 1, 2018

asolntsev added a commit that referenced this issue Oct 1, 2018

asolntsev added a commit that referenced this issue Oct 1, 2018

@asolntsev asolntsev referenced this issue Oct 1, 2018

Merged

$ and $$ should not open a new browser #823

2 of 3 tasks complete

asolntsev added a commit that referenced this issue Oct 1, 2018

#809 catch explicitly only needed exceptions
... instead of `Throwable` which is too generic. It doesn't make sence to wait for 4 seconds in case of IllegalStateException etc.

asolntsev added a commit that referenced this issue Oct 2, 2018

asolntsev added a commit that referenced this issue Oct 2, 2018

asolntsev added a commit that referenced this issue Oct 3, 2018

Merge pull request #828 from codeborne/report-closed-driver-exception
#809 getWebDriver() reports clear error message if driver has been closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment