Remove Automatic Creation of FireFox Browser When No WebDriver Bound

cmkatarn commented 6 years ago

The problem

Could you please add a check to the property "selenide.reopenBrowserOnFail" in WebDriverThreadLocalContainer.getWebDriver()? Line 95 has a log message that "No webdriver is bound to the current thread", followed by line 96 which creates a new driver.

Details

We happen to manage our own browser lifecycles (using WebDriverRunner.setWebDriver()) and run all of our tests on an internal hub/nodes, and would prefer no browser to be created if no webdriver is bound to the current thread. Currently, if we declare and initialize Selenide Page Objects outside of our test methods we are experiencing FireFox browsers being opened locally, even when running remotely. These browsers are never utilized as our tests use TestNG annotations (@BeforeClass) to set the WebDriver to a new instance of whatever type we've specified in our configuration files.

With the existing code, there seems to be no way to prevent the creation of a new FireFox browser, even if our tests handle the creation internally.

Code To Reproduce Issue

public class SomeClass {

//this will spawn a FF browser - it should not if reopenBrowserOnFail is 'false' private static SomePageObject somePageObject = page(SomePageObject.class);

@Test public void someTest() { //this will also spawn a FF browser - it should not if reopenBrowserOnFail is 'false' $("").click(); }

BorisOsipov commented 6 years ago

Hi, @cmkatarn, thank you for feedback.

I don't understand what we should do in your example

@test public void someTest() {
//this will also spawn a FF browser - it should not if reopenBrowserOnFail is 'false' reopenbrowser
$("").click();
}

What will happen when I call click() and there is no driver? throw exception? It is ridiculous..

asolntsev commented 6 years ago

Can you explain what is expected behavior for you? What should Selenide do if you try to create a page object, but haven't initialized browser yet?

On Feb 15, 2018 18:56, "cmkatarn" notifications@github.com wrote:

The problem

Could you please add a check to the property "selenide.reopenBrowserOnFail" in WebDriverThreadLocalContainer.getWebDriver()? Line 95 has a log message that "No webdriver is bound to the current thread", followed by line 96 which creates a new driver. Details

We happen to manage our own browser lifecycles (using WebDriverRunner.setWebDriver()) and run all of our tests on an internal hub/nodes, and would prefer no browser to be created if no webdriver is bound to the current thread. Currently, if we declare and initialize Selenide Page Objects outside of our test methods we are experiencing FireFox browsers being opened locally, even when running remotely. These browsers are never utilized as our tests use TestNG annotations (@beforeclass) to set the WebDriver to a new instance of whatever type we've specified in our configuration files.

With the existing code, there seems to be no way to prevent the creation of a new FireFox browser, even if our tests handle the creation internally. Code To Reproduce Issue

public class SomeClass {

//this will spawn a FF browser - it should not if reopenBrowserOnFail is 'false' private static SomePageObject somePageObject = page(SomePageObject.class);

@test https://github.com/test public void someTest() { //this will also spawn a FF browser - it should not if reopenBrowserOnFail is 'false' reopenbrowser$("").click(); }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/codeborne/selenide/issues/695, or mute the thread https://github.com/notifications/unsubscribe-auth/AARE3aTQxvLDKXJ4_VrQs0tdSNjx8cjxks5tVGHVgaJpZM4SHKxx .

cmkatarn commented 6 years ago

@BorisOsipov The creation of a WebDriver should be the responsibility of the user/author. If they would like to disable the automatic creation of a WebDriver due to their own handling of that creation, then they should be able to do that. We currently handle webdriver creation in a @BeforeClass extended by all of our test Classes, and so in the instance that a test class gets merged with initialization of a Page Object BEFORE the @BeforeClass, we end up with an extra local FireFox browser, even when we've specified we want to use Chrome, and even if we're running remotely. By leveraging a System property to determine whether or not to create a new webdriver and default it to FireFox, the responsibility falls on the engineer.

@asolntsev If we are creating a Page Object but have not yet created a webdriver, we should make a check against a System Property to determine our behavior. Ideally, that System Property would default to 'true' so that we do not impact existing tests, but that way the engineer can decide what happens in this instance, rather than having a local FireFox browser forced onto them.

asolntsev commented 6 years ago

I still don't understand. Ok, Selenide will NOT open Firefox, but what it should do? Throw an exception?

If so, you will need to fix your tests anyway.

On Feb 15, 2018 21:04, "cmkatarn" notifications@github.com wrote:

@BorisOsipov https://github.com/borisosipov The creation of a WebDriver should be the responsibility of the user/author. If they would like to disable the automatic creation of a WebDriver due to their own handling of that creation, then they should be able to do that. We currently handle webdriver creation in a @beforeclass extended by all of our test Classes, and so in the instance that a test class gets merged with initialization of a Page Object BEFORE the @beforeclass, we end up with an extra local FireFox browser, even when we've specified we want to use Chrome, and even if we're running remotely. By leveraging a System property to determine whether or not to create a new webdriver and default it to FireFox, the responsibility falls on the engineer.

@asolntsev https://github.com/asolntsev If we are creating a Page Object but have not yet created a webdriver, we should make a check against a System Property to determine our behavior. Ideally, that System Property would default to 'true' so that we do not impact existing tests, but that way the engineer can decide what happens in this instance, rather than having a local FireFox browser forced onto them.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codeborne/selenide/issues/695#issuecomment-366028551, or mute the thread https://github.com/notifications/unsubscribe-auth/AARE3X8L3P1a5M1n_EnAcV8C8j1ASSrSks5tVH-1gaJpZM4SHKxx .

cmkatarn commented 6 years ago

@asolntsev Requiring the engineer to fix the test is preferable to having the desktop of whoever is running the suite of tests hijacked by occasional local instances of FireFox.

Assuming the use of a System Property variable, either solution I had in mind (either return 'null' or throw an exception) would result in the test failing, but the Agent running the suite could continue with whatever they're doing unimpeded.

pashidlos commented 6 years ago

Looks like I have the same problem and it's related to TestNG with DataProvider run in parallel:

when you create driver in @BeforeTest/@BeforeClass method it's created in one thread
test that suppose to use this driver is run in another thread
Selenide cannot get already instantiated driver from different thread because thread id seams different

Cannot be reproduced:

without parallel execution
if you create driver specifically in test

Is this really possible to fix?

Code to reproduce

@BeforeTest
public void setUp() {
    WebDriver driver = new ChromeDriver();
    WebDriverRunner.setWebDriver(driver);
}

@DataProvider(name = "bar", parallel = true)
public Object[][] bar() {
    return new Object[][]{
            {
                    "http://www.google.com",
            },
    };
}

@Test(dataProvider = "bar")
public void foo(String url){
    Selenide.open(url);
}

selenide: 4.10.01 testng: 6.14.2 guava: 23.0

Check screenshoot There is driver already created but getAndCheckWebDriver() method decide to create another one and that is why you will see another browser instance (FF is default one)

ken-p commented 6 years ago

Hello - this was already partially implemented in issues 244 and 433. I like cmkatarn's suggestion and it would be very helpful for us. Perhaps throw an unsupported operation exception? The logic being, Selenide was asked to do something with the browser, a webdriver is not bound to the current thread, and also Selenide was told to not spawn a browser (via the selenide.reopenBrowserOnFail property).

asolntsev commented 6 years ago

@ken-p Yes, definitely, we should implement @cmkatarn suggestion. I will do it.

But it will NOT solve the issue described by @pashidlos.

@pashidlos I recommend you to implement WebDriverProvider instead of calling method WebDriverRunner.setWebDriver(). Then Selenide will open the browser at the right moment in the right thread. See https://github.com/codeborne/selenide/blob/master/src/test/java/integration/CustomWebDriverProviderTest.java for example.

asolntsev commented 6 years ago

Fixed by https://github.com/codeborne/selenide/pull/754

selenide / selenide