Can't use SelenideElement if it was declared before setWebDriver() method #873

pavelpp · 2018-12-05T10:42:03Z

The problem

As per issue title, declaring a SelenideElement before calling WebDriverRunner.setWebDriver(driver) method leads to an IllegalStateException whenever user tries to interact with the element.

Details

If we do not use setWebDriver method, then everything works regardless of where you declare SelenideElement. It can be a test class field for instance and still works. Exception stacktrace

java.lang.IllegalStateException: Webdriver has been closed. You need to call open(url) to open a browser again.

	at com.codeborne.selenide.drivercommands.LazyDriver.getWebDriver(LazyDriver.java:65)
	at com.codeborne.selenide.impl.ElementFinder.getSearchContext(ElementFinder.java:86)
	at com.codeborne.selenide.impl.ElementFinder.getWebElement(ElementFinder.java:74)
	at com.codeborne.selenide.impl.WebElementSource.checkCondition(WebElementSource.java:47)
	at com.codeborne.selenide.commands.Should.should(Should.java:35)
	at com.codeborne.selenide.commands.Should.execute(Should.java:29)
	at com.codeborne.selenide.commands.Should.execute(Should.java:12)
	at com.codeborne.selenide.commands.Commands.execute(Commands.java:144)
	at com.codeborne.selenide.impl.SelenideElementProxy.dispatchAndRetry(SelenideElementProxy.java:99)
	at com.codeborne.selenide.impl.SelenideElementProxy.invoke(SelenideElementProxy.java:65)
	at com.sun.proxy.$Proxy5.shouldBe(Unknown Source)

Tell us about your environment

Selenide Version: 5.0.1
Chrome\Firefox\IE Version: irrelevant
Browser Driver Version: irrelevant
Selenium Version: 3.x
OS Version: irrelevant

Code To Reproduce Issue [ Good To Have ]

Easily reproducible with:

        SelenideElement a=$("abc");
        WebDriverRunner.setWebDriver(new ChromeDriver());
        open("http://google.com");
        a.shouldNot(exist);

And this way it works

        WebDriverRunner.setWebDriver(new ChromeDriver());
        SelenideElement a=$("abc");
        open("http://google.com");
        a.shouldNot(exist);

Also this works

public class SimpleTest {

    public SelenideElement search = $(By.name("q"));

    @Test
    public void testSimple() {
        open("https://www.google.com");
        search.shouldBe(Condition.visible);
    }

}

And this does not

public class SimpleTest {

    public SelenideElement search = $(By.name("q"));

    @Test
    public void testSimple() {
        WebDriverRunner.setWebDriver(new ChromeDriver(new ChromeOptions()));
        open("https://www.google.com");
        search.shouldBe(Condition.visible);
    }
}

The text was updated successfully, but these errors were encountered:

jkromski · 2018-12-06T10:22:56Z

From what I understand, by using $() you setting up a default driver and $ uses it to declare SelenideElement, so if you're setting driver manually after $ then Selenide closes previous one and that is why you get the error.

SelenideElement a=$("abc"); // default driver (created for you automatically)
WebDriverRunner.setWebDriver(new ChromeDriver()); // your driver, default driver being closed
open("http://google.com"); // using your driver
a.shouldNot(exist); // as this is proxy it remembers default driver from line 1

I think this is not an bug, you should set driver before using it. if you need to declare element before then declare selector only as String and after setting up the driver use $()

vinogradoff · 2018-12-06T10:38:22Z

$() is not supposed to set up any particular driver, it just making use of a drivers configured at other places. So I guess it is a bug) Viele Grüße Alexei Vinogradov

…

On 6. December 2018 at 11:22:58, Jacek ***@***.***) wrote: From what I understand, by using $() you setting up a default driver and $ uses it to declare SelenideElement, so if you're setting driver manually after $ then Selenide closes previous one and that is why you get the error. SelenideElement a=$("abc"); // default driver (created for you automatically)WebDriverRunner.setWebDriver(new ChromeDriver()); // your driver, default driver being closed open("http://google.com"); // using your driver a.shouldNot(exist); // as this is proxy it remembers default driver from line 1 I think this is not an bug, you should set driver before using it. if you need to declare element before then declare selector only as String and after setting up the driver use $() — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#873 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGQ4YI_i4aYdKRKwlwNdTCwLL41QblZZks5u2PACgaJpZM4ZCdXD> .

pavelpp · 2018-12-06T10:41:39Z

@jkromski that would have been true if it didn't work without using setWebDriver() method, letting Selenide setup driver, but it's not the case. The issue is only happening when setWebDriver method is used. See my example above without usage of setWebDriver, where element is declared as a field - this still works.

vinogradoff · 2018-12-06T10:45:23Z

Actually, I've looked into code and it looks like bug. I hope to fix it until Sunday. Viele Grüße Alexei Vinogradov

…

On 6. December 2018 at 11:41:40, pavelpp ***@***.***) wrote: @jkromski <https://github.com/jkromski> that would have been true if it didn't work without using setWebDriver() method, letting Selenide setup driver, but it's not the case. The issue is only happening when setWebDriver method is used. See my example above without usage of setWebDriver, where element is declared as a field - this still works. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#873 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGQ4YGlQdWzwKCbtrMcn9vkhsFYxj1qVks5u2PRkgaJpZM4ZCdXD> .

asolntsev · 2018-12-12T22:33:07Z

@pavelpp @jkromski @vinogradoff No, it's not a bug. It was a decision made in Selenide 5.0. See https://selenide.org/2018/10/10/selenide-5.0.0/:

Methods $ and $$ now throw an exception if browser is not opened yet (or is already closed). The right way is to open a browser first (by calling open(url)) and then search elements ($, $$ etc).

It may see a bit unexpected, but I believe it will cause a better design of your tests. For me, it's really a strange decision to declare SelenideElement as a field in test class. Let's initialise it when it's needed.

pavelpp · 2018-12-12T22:44:50Z

It may seem a bit unexpected

It is indeed unexpected as it used to work before and not anymore. Secondly, if it's not supposed to work like this then the behaviour is not consistent when setWebDriver method is not used. How do you explain this? @asolntsev Make it work the same way in both cases, otherwise it just does not make any sense.

asolntsev · 2018-12-12T22:52:36Z

@pavelpp Well, I agree, Selenide should throw IllegalStateException even if setWebDriver was not used. I will do it in version 5.1, thanks for the suggestion.

P.S. But it will not solve your problem. :)

pavelpp · 2018-12-12T22:53:38Z

@asolntsev it's fine as long as the behaviour is consistent

symonk · 2018-12-13T09:19:36Z

Agree, lets improve what we are telling the user under this circumstance, but I think its a very uncommon to write test code like this

vinogradoff · 2018-12-13T11:35:57Z

I still see it as a bug. I will propose a fix later this week.

asolntsev · 2018-12-13T14:36:30Z

@vinogradoff before you start fixing something, could you explain why you think it's a bug? It was an intentional change.

vinogradoff · 2018-12-13T16:02:48Z

Sure.

This fails.

        SelenideElement a=$("abc");
        WebDriverRunner.setWebDriver(new ChromeDriver());
        open("http://google.com");
        a.shouldNot(exist);

This works

        WebDriverRunner.setWebDriver(new ChromeDriver());
        SelenideElement a=$("abc");
        open("http://google.com");
        a.shouldNot(exist);

WebDriver is changed before open - perfectly fine. There is no reason to connect elements definition with any WebDriver instance, so it may happen in any place. And as far as I remember it is not even connected, but doesn't work as a side effect due to some other minor issue in setWebDriver calls.

asolntsev · 2018-12-13T17:10:37Z

And where is the real problem?
Aren't you agree that much better is to first open a browser and then find elements?

vinogradoff · 2018-12-13T19:57:26Z

I still insist that the line doesn't search and doesn't find. It declares location algorithm. Declaration can be placed anywhere.

This example is of course just a short snippet to reproduce.

asolntsev · 2018-12-13T23:31:13Z

@vinogradoff Who said that? Why? I never said it. $ just loads elements lazily.

Anyway, my question is still in the air:

And where is the real problem? (snippet above can be easily fixed: just move $ below setWebDriver. I don't see any problems here.)
Aren't you agree that much better is to first open a browser and then find elements?

vinogradoff · 2018-12-13T23:57:54Z

There is a better solution - to make a code change and let the both snippets to have the same result.

asolntsev · 2018-12-14T00:01:23Z

You are not answering my questions. :(

vinogradoff · 2018-12-14T11:57:32Z

If you insist.

Who said that? Why? I never said it

I was never saying it was you. I don't know who says it except me. Why - because it work this way.

And where is the real problem?

The different behaviour of the two snippets is illogical. It is a real problem with quality, yes. That you have workaround doesn't make the problem less real.

Aren't you agree that much better is to first open a browser and then find elements?

We cannot find element before opening the browser, for sure. But even if you put "find" and "declare" to be equal, and even if I would agree that it is better to declare element after open() and not before. Just look again at this two snippets. They both declare element before open(), but one of the is working and the other isn't.

Then please answer my question in return:

Why you resist a change that would make these both pieces of code work consistently? What are the reasons not to do it? (of course you must not do it yourself).

asolntsev · 2018-12-14T19:24:44Z

@vinogradoff of course, inconsistency is bad. As I said in a comment above, Selenide should throw IllegalStateException in both cases.

If you want to declare elements, just declare By username = By.name("username");. Why are you calling it a hack? It's not a hack, it's a totally valid code.

vinogradoff · 2018-12-14T20:33:44Z

Why are you calling a hack? It's not a hack, it's a totally valid code.

Where I called By username = By.name("username"); a hack?

If you want to declare elements, just declare By

Why? Why you don't want me to declare SelenideElement as I always do the last years?

vinogradoff · 2018-12-14T20:37:09Z

Selenide should throw IllegalStateException in both cases

just beware, that if you throw IllegalStateException every time, $() is used, bevor open() is called - it will break plenty of tests, probably my tests too. For what reason?

BlackwellQA · 2018-12-24T18:26:22Z

This is indeed a bug. It prevents me from upgrading to Selenide 5+
I kept getting no webdriver is bound to current thread or webdriver is closed errors and couldn't understand why.

Well, @asolntsev said

And where is the real problem?
Aren't you agree that much better is to first open a browser and then find elements?

I have collection of elements for each page on top of my java class (like 800+ of them), they're all organized and commented. They are being used in MULTIPLE tests below. It is way easier and makes more sense to create an array of elements of the page first, then use them where needed in tests.

It shouldn't have anything to do with browser setup, it also has nothing to do with $ and $$ opening browsers. It's just a collection of elements and it shouldn't affect setup and tests execution.

asolntsev · 2019-02-06T20:52:46Z

@BlackwellQA Well, it was not a bug, it was an intentional change because I believed it will cause better design of tests. :)
But now I see that it causes a big inconvenience for you (and not only you), so I should probably change my mind. :)

But still, don't you think that having 800+ fields in a single class is a bad design? I mean, BAD design? It breaks all principles of software disciplines like SRP, KISS etc. A big class with many responsibilities is a well-known antipattern in programming (not only in test automation).

Don't you think that splitting the huge class to many small page objects would cause a much better design?

BlackwellDev · 2019-02-06T21:21:16Z

@asolntsev Thank you for your reply Andrei :)

I managed to get everything working by adding By and $() everywhere, which is not bad but more things to type when with just elements collection looks a bit cleaner :) The error was very confusing and I tried to rewrite my browser factory and everything, but issue was that elements were before page opening separated from the tests.

Design wise I understand where you're coming from, but in my specific case I do DNA traits and indexes automation and they all have to be together in one class unfortunately.

ivalitov · 2019-03-11T22:06:00Z

I preapare instances of page objects before running parameterised tests via Juni5 ArgumentsProvider. Page objects already includes fields and actions with $. Next browser is selected before each test and if browser changes from default, it produses that problem..

Actualy, it wasn't a big problem. I've just moved invocation of page object into test.

Skaramus1990 · 2019-05-15T08:40:13Z

Hi, I have the same problem.
I using Spring Boot in my tests. When I run tests in many threads, I have this exception. Only 1 from 5 thread works fine. If I added in my object with elements @scope(value =ConfigurableBeanFactory.SCOPE_PROTOTYPE, proxyMode = ScopedProxyMode.TARGET_CLASS) all works fine. Or if I write = new MyObject(); before calling element's method.

Skaramus1990 · 2019-05-15T09:01:54Z

In @scope the interfaces was added proxyMode = ScopedProxyMode.TARGET_CLASS. It solved my problem.

EXAMPLE:

Target (TYPE)
@retention (RetentionPolicy.RUNTIME)
@component
@scope ("thread", proxyMode = ScopedProxyMode.TARGET_CLASS)
@inherited
public @interface PageObject {}

urbaniakmichal · 2019-06-02T11:46:09Z

I have the same problem:

@before(order = 1) - I use properies file (only string like user name, pass etc)
@before(order = 2) - I set configuration like timeout
@before(order = 3) I open url and I for the first time use driver
next I try find $("bla bla bla") and And I am getting :

java.lang.IllegalStateException: Webdriver has been closed. You need to call open(url) to open a browser again.

@before(order = 1)
public void initProperties() throws ReadPropertiesFileException {
propertiesConfig = new PropertiesConfig();
}

@Before(order = 2)
public void setConfiguration() {
    Configuration.fastSetValue = true;
    Configuration.screenshots = true;
    Configuration.timeout = 15000;
    Configuration.reportsFolder = "reports/screens";
}

@Before(order = 3)
public void openUrl() {
    System.setProperty("webdriver.gecko.driver", "src/test/resources/drivers/geckodriver0240.exe");
    WebDriverRunner.setWebDriver(new FirefoxDriver());
    open(propertiesConfig.getDemoUrl());
    getWebDriver().manage().deleteAllCookies();

}

After(in different class) I want to use:

public class LoginPage {
private String navbarSelector = ".nav.navbar-nav.hidden-sm.go-left";
private SelenideElement myAccountDropdown = $(navbarSelector).$$("li").first();
private SelenideElement loginOption = $(navbarSelector).$(".dropdown-menu").$$("li").first().$(".go-text-right");
private SelenideElement demoUserEmail = $x("//input[@type='email']");
private SelenideElement demoUserPassword = $x("//input[@type='password']");

public LoginPage openMyAccountDropdown() {
    myAccountDropdown.click();

    return this;
}

And I am getting :

java.lang.IllegalStateException: Webdriver has been closed. You need to call open(url) to open a browser again. at com.codeborne.selenide.drivercommands.LazyDriver.getWebDriver(LazyDriver.java:64) at com.codeborne.selenide.impl.ElementFinder.getSearchContext(ElementFinder.java:86) at com.codeborne.selenide.impl.ElementFinder.getWebElement(ElementFinder.java:74) at com.codeborne.selenide.impl.SelenideElementProxy.dispatchAndRetry(SelenideElementProxy.java:102) at com.codeborne.selenide.impl.SelenideElementProxy.invoke(SelenideElementProxy.java:65) at com.sun.proxy.$Proxy4.findElements(Unknown Source) at com.codeborne.selenide.impl.WebElementSelector.findElements(WebElementSelector.java:37) at com.codeborne.selenide.impl.BySelectorCollection.getElements(BySelectorCollection.java:30) at com.codeborne.selenide.impl.CollectionElement.getWebElement(CollectionElement.java:35) at com.codeborne.selenide.impl.WebElementSource.checkCondition(WebElementSource.java:50) at com.codeborne.selenide.impl.WebElementSource.findAndAssertElementIsInteractable(WebElementSource.java:87) at com.codeborne.selenide.commands.Click.execute(Click.java:13) at com.codeborne.selenide.commands.Click.execute(Click.java:9) at com.codeborne.selenide.commands.Commands.execute(Commands.java:144) at com.codeborne.selenide.impl.SelenideElementProxy.dispatchAndRetry(SelenideElementProxy.java:99) at com.codeborne.selenide.impl.SelenideElementProxy.invoke(SelenideElementProxy.java:65) at com.sun.proxy.$Proxy4.click(Unknown Source) at pageobject.login.LoginPage.openMyAccountDropdown(LoginPage.java:23) at glue.GenericDef.lambda$new$0(GenericDef.java:12) at ✽.I am logged to app(file:/D:/Workspace/intelij/phptravels/src/test/resources/features/smoke/smokeLanguage.feature:8)

asolntsev · 2019-10-28T23:38:58Z

@pavelpp @jkromski @BlackwellQA @ivalitov @Skaramus1990 @MichalUrbaniakQA Congratulations, this change was implemented in Selenide 5.4.1.

asolntsev self-assigned this Dec 12, 2018

asolntsev added the not a bug label Dec 12, 2018

asolntsev added this to the 5.1.0 milestone Dec 12, 2018

asolntsev removed the not a bug label Dec 12, 2018

vinogradoff closed this as completed Dec 13, 2018

vinogradoff reopened this Dec 13, 2018

rosolko modified the milestones: 5.1.0, 5.1.1 Dec 17, 2018

asolntsev added not sure not a bug labels Feb 6, 2019

asolntsev removed this from the 5.1.1 milestone Feb 6, 2019

This was referenced Mar 1, 2019

NPE on opening WebDriver to remote Selenium HUB #887

Closed

JSONException when trying to upgrade to Selenide 5.2.0 with Selenium Grid 2 version 2.53 #916

Closed

asolntsev added this to the 5.4.0 milestone Oct 28, 2019

asolntsev closed this as completed Oct 28, 2019

asolntsev removed the not sure label Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Can't use SelenideElement if it was declared before setWebDriver() method #873

Can't use SelenideElement if it was declared before setWebDriver() method #873

pavelpp commented Dec 5, 2018

jkromski commented Dec 6, 2018

vinogradoff commented Dec 6, 2018 via email

pavelpp commented Dec 6, 2018

vinogradoff commented Dec 6, 2018 via email

asolntsev commented Dec 12, 2018

pavelpp commented Dec 12, 2018 •

edited

asolntsev commented Dec 12, 2018

pavelpp commented Dec 12, 2018

symonk commented Dec 13, 2018

vinogradoff commented Dec 13, 2018 via email

asolntsev commented Dec 13, 2018

vinogradoff commented Dec 13, 2018 •

edited

asolntsev commented Dec 13, 2018

vinogradoff commented Dec 13, 2018 •

edited

asolntsev commented Dec 13, 2018

vinogradoff commented Dec 13, 2018

asolntsev commented Dec 14, 2018

vinogradoff commented Dec 14, 2018

asolntsev commented Dec 14, 2018

vinogradoff commented Dec 14, 2018

vinogradoff commented Dec 14, 2018 •

edited

BlackwellQA commented Dec 24, 2018

asolntsev commented Feb 6, 2019

BlackwellDev commented Feb 6, 2019

ivalitov commented Mar 11, 2019 •

edited

Skaramus1990 commented May 15, 2019 •

edited

Skaramus1990 commented May 15, 2019

urbaniakmichal commented Jun 2, 2019 •

edited

asolntsev commented Oct 28, 2019

Can't use SelenideElement if it was declared before setWebDriver() method #873

Can't use SelenideElement if it was declared before setWebDriver() method #873

Comments

pavelpp commented Dec 5, 2018

The problem

Details

Tell us about your environment

Code To Reproduce Issue [ Good To Have ]

jkromski commented Dec 6, 2018

vinogradoff commented Dec 6, 2018 via email

pavelpp commented Dec 6, 2018

vinogradoff commented Dec 6, 2018 via email

asolntsev commented Dec 12, 2018

pavelpp commented Dec 12, 2018 • edited

asolntsev commented Dec 12, 2018

pavelpp commented Dec 12, 2018

symonk commented Dec 13, 2018

vinogradoff commented Dec 13, 2018 via email

asolntsev commented Dec 13, 2018

vinogradoff commented Dec 13, 2018 • edited

asolntsev commented Dec 13, 2018

vinogradoff commented Dec 13, 2018 • edited

asolntsev commented Dec 13, 2018

vinogradoff commented Dec 13, 2018

asolntsev commented Dec 14, 2018

vinogradoff commented Dec 14, 2018

asolntsev commented Dec 14, 2018

vinogradoff commented Dec 14, 2018

vinogradoff commented Dec 14, 2018 • edited

BlackwellQA commented Dec 24, 2018

asolntsev commented Feb 6, 2019

BlackwellDev commented Feb 6, 2019

ivalitov commented Mar 11, 2019 • edited

Skaramus1990 commented May 15, 2019 • edited

Skaramus1990 commented May 15, 2019

urbaniakmichal commented Jun 2, 2019 • edited

asolntsev commented Oct 28, 2019

pavelpp commented Dec 12, 2018 •

edited

vinogradoff commented Dec 13, 2018 •

edited

vinogradoff commented Dec 13, 2018 •

edited

vinogradoff commented Dec 14, 2018 •

edited

ivalitov commented Mar 11, 2019 •

edited

Skaramus1990 commented May 15, 2019 •

edited

urbaniakmichal commented Jun 2, 2019 •

edited