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

Remove Automatic Creation of FireFox Browser When No WebDriver Bound #695

Closed
cmkatarn opened this Issue Feb 15, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@cmkatarn

cmkatarn commented Feb 15, 2018

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

This comment has been minimized.

Member

BorisOsipov commented Feb 15, 2018

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

This comment has been minimized.

Contributor

asolntsev commented Feb 15, 2018

@cmkatarn

This comment has been minimized.

cmkatarn commented Feb 15, 2018

@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

This comment has been minimized.

Contributor

asolntsev commented Feb 15, 2018

@cmkatarn

This comment has been minimized.

cmkatarn commented Feb 15, 2018

@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

This comment has been minimized.

pashidlos commented Feb 16, 2018

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)

1

@ken-p

This comment has been minimized.

ken-p commented Feb 23, 2018

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

This comment has been minimized.

Contributor

asolntsev commented Jun 5, 2018

@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 asolntsev self-assigned this Jun 5, 2018

asolntsev added a commit that referenced this issue Jun 5, 2018

#695 Do not open a browser if `Configuration.reopenBrowserOnFail` is …
…`false` and user has not set webdriver manually

@asolntsev asolntsev added this to the 4.12.2 milestone Jun 5, 2018

asolntsev added a commit that referenced this issue Jun 21, 2018

Merge pull request #754 from codeborne/do-not-open-browser
#695 Do not open a browser if `Configuration.reopenBrowserOnFail` is …
@asolntsev

This comment has been minimized.

Contributor

asolntsev commented Jun 21, 2018

Fixed by #754

@asolntsev asolntsev closed this Jun 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment