Skip to content

Commit

Permalink
#896 Do close the browser in SelenideDriver.close()
Browse files Browse the repository at this point in the history
this method didn't close user-provided webdriver - and it caused butthurt for many users.
  • Loading branch information
asolntsev committed Oct 15, 2019
1 parent 74299d7 commit a280795
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 5 deletions.
Expand Up @@ -8,6 +8,7 @@
import org.testng.internal.Nullable;

import javax.annotation.Nonnull;
import java.util.logging.Logger;

import static java.util.Objects.requireNonNull;

Expand All @@ -17,17 +18,26 @@
* It doesn't start a new proxy.
*/
public class WebDriverWrapper implements Driver {
private static final Logger log = Logger.getLogger(WebDriverWrapper.class.getName());

private final Config config;
private final WebDriver webDriver;
private final SelenideProxyServer selenideProxy;
private final BrowserHealthChecker browserHealthChecker;

public WebDriverWrapper(@Nonnull Config config, @Nonnull WebDriver webDriver, @Nullable SelenideProxyServer selenideProxy) {
this(config, webDriver, selenideProxy, new BrowserHealthChecker());
}

WebDriverWrapper(@Nonnull Config config, @Nonnull WebDriver webDriver, @Nullable SelenideProxyServer selenideProxy,
@Nonnull BrowserHealthChecker browserHealthChecker) {
requireNonNull(config, "config must not be null");
requireNonNull(webDriver, "webDriver must not be null");

this.config = config;
this.webDriver = webDriver;
this.selenideProxy = selenideProxy;
this.browserHealthChecker = browserHealthChecker;
}

@Override
Expand Down Expand Up @@ -57,14 +67,25 @@ public SelenideProxyServer getProxy() {

@Override
public WebDriver getAndCheckWebDriver() {
if (webDriver != null && !browserHealthChecker.isBrowserStillOpen(webDriver)) {
log.info("Webdriver has been closed meanwhile");
close();
return null;
}
return webDriver;
}

/**
* Does not close webdriver.
* This class holds a webdriver created by user - in this case user is responsible for closing webdriver by himself.
* Close the webdriver.
*
* NB! The behaviour was changed in Selenide 5.4.0
* Even if webdriver was created by user - it will be closed.
* It may hurt if you try to use this browser after closing.
*/
@Override
public void close() {
if (!config.holdBrowserOpen()) {
new CloseDriverCommand(webDriver, selenideProxy).run();
}
}
}
Expand Up @@ -5,17 +5,19 @@
import org.openqa.selenium.WebDriver;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

class WebDriverWrapperTest {

@Test
void name() {
void close_closesTheBrowser() {
WebDriver webDriver = mock(WebDriver.class);
WebDriverWrapper driver = new WebDriverWrapper(new SelenideConfig(), webDriver, null);

driver.close();

verify(webDriver).quit();
verifyNoMoreInteractions(webDriver);
}
}
3 changes: 1 addition & 2 deletions src/test/java/integration/FirefoxWithProfileTest.java
Expand Up @@ -29,8 +29,7 @@ void setUp() {
@AfterEach
void tearDown() {
if (customFirefox != null) {
customFirefox.getWebDriver().close(); // change to customFirefox.close() after Bugfix is merged
// bug: SelenideDriver.close() doesn't close the browser
customFirefox.close();
}
}

Expand Down

0 comments on commit a280795

Please sign in to comment.