From ca1467eae6346a56397949e0a019512b9b90e4d3 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Tue, 28 Aug 2018 15:10:39 +0300 Subject: [PATCH 1/2] #788 Add setting to enable/disable proxy server --- CHANGELOG.md | 1 + .../com/codeborne/selenide/Configuration.java | 29 +++++ .../selenide/commands/DownloadFile.java | 24 ++-- .../codeborne/selenide/impl/Navigator.java | 5 +- .../impl/WebDriverThreadLocalContainer.java | 8 +- .../proxy/BrowserMobProxyServerUnlimited.java | 25 ++++ .../selenide/proxy/InetAddressResolver.java | 15 +++ .../selenide/proxy/SelenideProxyServer.java | 58 +++++---- .../selenide/WebDriverRunnerTest.java | 2 - .../selenide/commands/DownloadFileTest.java | 114 ++++++++++++++++++ .../selenide/impl/NavigatorTest.java | 14 +-- .../WebDriverThreadLocalContainerTest.java | 59 ++++----- .../proxy/InetAddressResolverStub.java | 16 +++ .../proxy/SelenideProxyServerTest.java | 73 +++++++---- src/test/java/grid/SeleniumGridTest.java | 3 +- .../java/integration/IntegrationTest.java | 2 + 16 files changed, 339 insertions(+), 109 deletions(-) create mode 100644 src/main/java/com/codeborne/selenide/proxy/BrowserMobProxyServerUnlimited.java create mode 100644 src/main/java/com/codeborne/selenide/proxy/InetAddressResolver.java create mode 100644 src/test/java/com/codeborne/selenide/commands/DownloadFileTest.java create mode 100644 src/test/java/com/codeborne/selenide/proxy/InetAddressResolverStub.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 95267363fa..7829cad497 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 4.14.0 (planned to xx.09.2018) * #784 Enable BasicAuth through Selenide proxy server -- see https://github.com/codeborne/selenide/pull/785 +* #788 Add setting to enable/disable proxy server * #789 Remove `?timestamp` parameter for IE ## 4.13.0 (released 20.08.2018) diff --git a/src/main/java/com/codeborne/selenide/Configuration.java b/src/main/java/com/codeborne/selenide/Configuration.java index 1fa6ddff08..3fbd08c12e 100644 --- a/src/main/java/com/codeborne/selenide/Configuration.java +++ b/src/main/java/com/codeborne/selenide/Configuration.java @@ -380,6 +380,35 @@ public enum FileDownloadMode { public static FileDownloadMode fileDownload = FileDownloadMode.valueOf( System.getProperty("selenide.fileDownload", HTTPGET.name())); + /** + * If Selenide should run browser through its own proxy server. + * It allows some additional features which are not possible with plain Selenium. + * But it's not enabled by default because sometimes it would not work (more exactly, if tests and browser and + * executed on different machines, and "test machine" is not accessible from "browser machine"). If it's not your + * case, I recommend to enable proxy. + * + * Default: false + */ + public static boolean proxyEnabled = Boolean.parseBoolean(System.getProperty("selenide.proxyEnabled", "false")); + + /** + * Host of Selenide proxy server. + * Used only if proxyEnabled == true. + * + * Default: empty (meaning that Selenide will detect current machine's ip/hostname automatically) + * + * @see net.lightbody.bmp.client.ClientUtil#getConnectableAddress() + */ + public static String proxyHost = System.getProperty("selenide.proxyHost", ""); + + /** + * Port of Selenide proxy server. + * Used only if proxyEnabled == true. + * + * Default: 0 (meaning that Selenide will choose a random free port on current machine) + */ + public static int proxyPort = Integer.parseInt(System.getProperty("selenide.proxyPort", "0")); + /** * Controls Selenide and WebDriverManager integration. * When integration is enabled you don't need to download and setup any browser driver executables. diff --git a/src/main/java/com/codeborne/selenide/commands/DownloadFile.java b/src/main/java/com/codeborne/selenide/commands/DownloadFile.java index 60a34b9cc2..06646b857b 100644 --- a/src/main/java/com/codeborne/selenide/commands/DownloadFile.java +++ b/src/main/java/com/codeborne/selenide/commands/DownloadFile.java @@ -10,7 +10,6 @@ import java.io.File; import java.io.IOException; -import java.util.Objects; import java.util.logging.Logger; import static com.codeborne.selenide.Configuration.FileDownloadMode.HTTPGET; @@ -19,8 +18,17 @@ public class DownloadFile implements Command { private static final Logger LOG = Logger.getLogger(DownloadFile.class.getName()); - DownloadFileWithHttpRequest downloadFileWithHttpRequest = new DownloadFileWithHttpRequest(); - DownloadFileWithProxyServer downloadFileWithProxyServer = new DownloadFileWithProxyServer(); + private final DownloadFileWithHttpRequest downloadFileWithHttpRequest; + private final DownloadFileWithProxyServer downloadFileWithProxyServer; + + public DownloadFile() { + this(new DownloadFileWithHttpRequest(), new DownloadFileWithProxyServer()); + } + + DownloadFile(DownloadFileWithHttpRequest httpget, DownloadFileWithProxyServer proxy) { + downloadFileWithHttpRequest = httpget; + downloadFileWithProxyServer = proxy; + } @Override public File execute(SelenideElement proxy, WebElementSource linkWithHref, Object[] args) throws IOException { @@ -32,18 +40,20 @@ public File execute(SelenideElement proxy, WebElementSource linkWithHref, Object LOG.config("selenide.fileDownload = " + System.getProperty("selenide.fileDownload") + " download file via http get"); return downloadFileWithHttpRequest.download(link, timeout); } + else if (!Configuration.proxyEnabled) { + throw new IllegalStateException("Cannot download file: proxy server is not enabled. Setup Configuration.proxyEnabled"); + } else if (webdriverContainer.getProxyServer() == null) { - LOG.config("Proxy server is not started - download file via http get"); - return downloadFileWithHttpRequest.download(link, timeout); + throw new IllegalStateException("Cannot download file: proxy server is not started"); } else { return downloadFileWithProxyServer.download(linkWithHref, link, webdriverContainer.getProxyServer(), timeout); } } - private long getTimeout(Object[] args) { + long getTimeout(Object[] args) { try { - if (Objects.nonNull(args) && args.length > 0) { + if (args != null && args.length > 0) { return (long) args[0]; } else { diff --git a/src/main/java/com/codeborne/selenide/impl/Navigator.java b/src/main/java/com/codeborne/selenide/impl/Navigator.java index 08aad424a0..0e99474f57 100644 --- a/src/main/java/com/codeborne/selenide/impl/Navigator.java +++ b/src/main/java/com/codeborne/selenide/impl/Navigator.java @@ -12,7 +12,6 @@ import java.net.URL; -import static com.codeborne.selenide.Configuration.FileDownloadMode.PROXY; import static com.codeborne.selenide.Configuration.baseUrl; import static com.codeborne.selenide.WebDriverRunner.getAndCheckWebDriver; import static com.codeborne.selenide.WebDriverRunner.getSelenideProxy; @@ -76,7 +75,7 @@ private void navigateTo(String url, AuthenticationType authenticationType, Strin } private void beforeNavigateTo(AuthenticationType authenticationType, String domain, String login, String password) { - if (Configuration.fileDownload == PROXY) { + if (Configuration.proxyEnabled) { beforeNavigateToWithProxy(authenticationType, domain, login, password); } else { @@ -111,7 +110,7 @@ private String appendBasicAuthIfNeeded(String url, AuthenticationType authType, private boolean passBasicAuthThroughUrl(AuthenticationType authenticationType, String domain, String login, String password) { return hasAuthentication(domain, login, password) && - Configuration.fileDownload != PROXY && + !Configuration.proxyEnabled && authenticationType == AuthenticationType.BASIC; } diff --git a/src/main/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainer.java b/src/main/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainer.java index 070f71a832..11c66b5e61 100644 --- a/src/main/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainer.java +++ b/src/main/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainer.java @@ -221,7 +221,13 @@ protected WebDriver createDriver() { Proxy userProvidedProxy = proxy; - if (Configuration.fileDownload == PROXY) { + if (!Configuration.proxyEnabled && Configuration.fileDownload == PROXY) { + log.warning("Configuration.proxyEnabled == false but Configuration.fileDownload == PROXY. " + + "We will enable proxy server automatically."); + Configuration.proxyEnabled = true; + } + + if (Configuration.proxyEnabled) { SelenideProxyServer selenideProxyServer = new SelenideProxyServer(proxy); selenideProxyServer.start(); THREAD_PROXY_SERVER.put(currentThread().getId(), selenideProxyServer); diff --git a/src/main/java/com/codeborne/selenide/proxy/BrowserMobProxyServerUnlimited.java b/src/main/java/com/codeborne/selenide/proxy/BrowserMobProxyServerUnlimited.java new file mode 100644 index 0000000000..e51fb4b4da --- /dev/null +++ b/src/main/java/com/codeborne/selenide/proxy/BrowserMobProxyServerUnlimited.java @@ -0,0 +1,25 @@ +package com.codeborne.selenide.proxy; + +import net.lightbody.bmp.BrowserMobProxyServer; +import net.lightbody.bmp.filters.RequestFilter; +import net.lightbody.bmp.filters.RequestFilterAdapter; +import net.lightbody.bmp.filters.ResponseFilter; +import net.lightbody.bmp.filters.ResponseFilterAdapter; + +/** + * By default, BrowserMobProxyServer doesn't allow requests/responses bugger than 2 MB. + * We need this class to enable bigger sizes. + */ +class BrowserMobProxyServerUnlimited extends BrowserMobProxyServer { + private static final int maxSize = 64 * 1024 * 1024; // 64 MB + + @Override + public void addRequestFilter(RequestFilter filter) { + addFirstHttpFilterFactory(new RequestFilterAdapter.FilterSource(filter, maxSize)); + } + + @Override + public void addResponseFilter(ResponseFilter filter) { + addLastHttpFilterFactory(new ResponseFilterAdapter.FilterSource(filter, maxSize)); + } +} diff --git a/src/main/java/com/codeborne/selenide/proxy/InetAddressResolver.java b/src/main/java/com/codeborne/selenide/proxy/InetAddressResolver.java new file mode 100644 index 0000000000..747278fa93 --- /dev/null +++ b/src/main/java/com/codeborne/selenide/proxy/InetAddressResolver.java @@ -0,0 +1,15 @@ +package com.codeborne.selenide.proxy; + +import java.net.InetAddress; +import java.net.UnknownHostException; + +public class InetAddressResolver { + InetAddress getInetAddressByName(String hostname) { + try { + return InetAddress.getByName(hostname); + } + catch (UnknownHostException e) { + throw new IllegalArgumentException(e); + } + } +} diff --git a/src/main/java/com/codeborne/selenide/proxy/SelenideProxyServer.java b/src/main/java/com/codeborne/selenide/proxy/SelenideProxyServer.java index 5dbaf7f364..cc7727385d 100644 --- a/src/main/java/com/codeborne/selenide/proxy/SelenideProxyServer.java +++ b/src/main/java/com/codeborne/selenide/proxy/SelenideProxyServer.java @@ -1,12 +1,10 @@ package com.codeborne.selenide.proxy; +import com.codeborne.selenide.Configuration; import net.lightbody.bmp.BrowserMobProxy; -import net.lightbody.bmp.BrowserMobProxyServer; import net.lightbody.bmp.client.ClientUtil; import net.lightbody.bmp.filters.RequestFilter; -import net.lightbody.bmp.filters.RequestFilterAdapter; import net.lightbody.bmp.filters.ResponseFilter; -import net.lightbody.bmp.filters.ResponseFilterAdapter; import org.openqa.selenium.Proxy; import java.net.InetSocketAddress; @@ -14,6 +12,7 @@ import java.util.Map; import static java.lang.Integer.parseInt; +import static org.apache.commons.lang3.StringUtils.isEmpty; /** * Selenide own proxy server to intercept server responses @@ -21,32 +20,12 @@ * It holds map of request and response filters by name. */ public class SelenideProxyServer { - protected final Proxy outsideProxy; - - protected BrowserMobProxy proxy = new BrowserMobProxyServer() { - int maxSize = 64 * 1024 * 1024; // 64 MB - @Override - public void addRequestFilter(RequestFilter filter) { - addFirstHttpFilterFactory(new RequestFilterAdapter.FilterSource(filter, maxSize)); - } - - @Override public void addResponseFilter(ResponseFilter filter) { - addLastHttpFilterFactory(new ResponseFilterAdapter.FilterSource(filter, maxSize)); - } - }; - - /** - * Method return current instance of browser mob proxy - * - * @return browser mob proxy instance - */ - public BrowserMobProxy getProxy() { - return proxy; - } - - protected int port; - protected Map requestFilters = new HashMap<>(); - protected Map responseFilters = new HashMap<>(); + private final InetAddressResolver inetAddressResolver; + private final Proxy outsideProxy; + private final BrowserMobProxy proxy; + private final Map requestFilters = new HashMap<>(); + private final Map responseFilters = new HashMap<>(); + private int port; /** * Create server @@ -55,7 +34,13 @@ public BrowserMobProxy getProxy() { * @param outsideProxy another proxy server used by test author for his own need (can be null) */ public SelenideProxyServer(Proxy outsideProxy) { + this(outsideProxy, new InetAddressResolver(), new BrowserMobProxyServerUnlimited()); + } + + protected SelenideProxyServer(Proxy outsideProxy, InetAddressResolver inetAddressResolver, BrowserMobProxy proxy) { this.outsideProxy = outsideProxy; + this.inetAddressResolver = inetAddressResolver; + this.proxy = proxy; } /** @@ -74,7 +59,7 @@ public void start() { addResponseFilter("responseSizeWatchdog", new ResponseSizeWatchdog()); addResponseFilter("download", new FileDownloadFilter()); - proxy.start(); + proxy.start(Configuration.proxyPort); port = proxy.getPort(); } @@ -121,7 +106,9 @@ static InetSocketAddress getProxyAddress(Proxy proxy) { * Converts this proxy to a "selenium" proxy that can be used by webdriver */ public Proxy createSeleniumProxy() { - return ClientUtil.createSeleniumProxy(proxy); + return isEmpty(Configuration.proxyHost) + ? ClientUtil.createSeleniumProxy(proxy) + : ClientUtil.createSeleniumProxy(proxy, inetAddressResolver.getInetAddressByName(Configuration.proxyHost)); } /** @@ -131,6 +118,15 @@ public void shutdown() { proxy.abort(); } + /** + * Method return current instance of browser mob proxy + * + * @return browser mob proxy instance + */ + public BrowserMobProxy getProxy() { + return proxy; + } + @Override public String toString() { return String.format("Selenide proxy server :%s", port); diff --git a/src/test/java/com/codeborne/selenide/WebDriverRunnerTest.java b/src/test/java/com/codeborne/selenide/WebDriverRunnerTest.java index 6083c8042b..0dec7616dd 100644 --- a/src/test/java/com/codeborne/selenide/WebDriverRunnerTest.java +++ b/src/test/java/com/codeborne/selenide/WebDriverRunnerTest.java @@ -16,7 +16,6 @@ import java.net.URL; -import static com.codeborne.selenide.Configuration.FileDownloadMode.HTTPGET; import static com.codeborne.selenide.Selenide.open; import static com.codeborne.selenide.WebDriverRunner.FIREFOX; import static com.codeborne.selenide.WebDriverRunner.HTMLUNIT; @@ -43,7 +42,6 @@ void resetWebDriverContainer() { WebDriverRunner.webdriverContainer = spy(new WebDriverThreadLocalContainer()); doReturn(null).when((JavascriptExecutor) driver).executeScript(anyString(), any()); - Configuration.fileDownload = HTTPGET; } @AfterEach diff --git a/src/test/java/com/codeborne/selenide/commands/DownloadFileTest.java b/src/test/java/com/codeborne/selenide/commands/DownloadFileTest.java new file mode 100644 index 0000000000..8860f64ea2 --- /dev/null +++ b/src/test/java/com/codeborne/selenide/commands/DownloadFileTest.java @@ -0,0 +1,114 @@ +package com.codeborne.selenide.commands; + +import com.codeborne.selenide.Configuration; +import com.codeborne.selenide.extension.MockWebDriverExtension; +import com.codeborne.selenide.impl.DownloadFileWithHttpRequest; +import com.codeborne.selenide.impl.DownloadFileWithProxyServer; +import com.codeborne.selenide.impl.WebElementSource; +import com.codeborne.selenide.proxy.SelenideProxyServer; +import org.assertj.core.api.WithAssertions; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.openqa.selenium.WebElement; + +import java.io.File; +import java.io.IOException; + +import static com.codeborne.selenide.Configuration.FileDownloadMode.HTTPGET; +import static com.codeborne.selenide.Configuration.FileDownloadMode.PROXY; +import static com.codeborne.selenide.WebDriverRunner.webdriverContainer; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockWebDriverExtension.class) +class DownloadFileTest implements WithAssertions { + private DownloadFileWithHttpRequest httpget = mock(DownloadFileWithHttpRequest.class); + private DownloadFileWithProxyServer proxy = mock(DownloadFileWithProxyServer.class); + private DownloadFile command = new DownloadFile(httpget, proxy); + private WebElementSource linkWithHref = mock(WebElementSource.class); + private WebElement link = mock(WebElement.class); + private File file = new File("some-file.yxy"); + + @BeforeEach + @AfterEach + void reset() { + Configuration.proxyEnabled = false; + Configuration.fileDownload = HTTPGET; + } + + @BeforeEach + void setUp() { + when(linkWithHref.findAndAssertElementIsVisible()).thenReturn(link); + } + + @Test + void canDownloadFile_withHttpGetRequest() throws IOException { + Configuration.proxyEnabled = false; + Configuration.fileDownload = HTTPGET; + + when(httpget.download(any(WebElement.class), anyLong())).thenReturn(file); + + File f = command.execute(null, linkWithHref, new Object[]{8000L}); + + assertThat(f).isSameAs(file); + verify(httpget).download(link, 8000L); + verifyNoMoreInteractions(proxy); + } + + @Test + void canDownloadFile_withProxyServer() throws IOException { + Configuration.proxyEnabled = true; + Configuration.fileDownload = PROXY; + SelenideProxyServer selenideProxy = mock(SelenideProxyServer.class); + doReturn(selenideProxy).when(webdriverContainer).getProxyServer(); + when(proxy.download(any(), any(), any(), anyLong())).thenReturn(file); + + File f = command.execute(null, linkWithHref, new Object[]{9000L}); + + assertThat(f).isSameAs(file); + verify(proxy).download(linkWithHref, link, selenideProxy, 9000L); + verifyNoMoreInteractions(httpget); + } + + @Test + void proxyServerShouldBeEnabled() { + Configuration.proxyEnabled = false; + Configuration.fileDownload = PROXY; + + assertThatThrownBy(() -> command.execute(null, linkWithHref, new Object[0])) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot download file: proxy server is not enabled"); + } + + @Test + void proxyServerShouldBeStarted() { + Configuration.proxyEnabled = true; + Configuration.fileDownload = PROXY; + doReturn(null).when(webdriverContainer).getProxyServer(); + + assertThatThrownBy(() -> command.execute(null, linkWithHref, new Object[0])) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot download file: proxy server is not started"); + } + + @Test + void defaultTimeout() { + Configuration.timeout = 3L; + + assertThat(command.getTimeout(new Object[0])).isEqualTo(3L); + } + + @Test + void customerTimeout() { + Configuration.timeout = 3L; + + assertThat(command.getTimeout(new Object[]{2L})).isEqualTo(2L); + } +} diff --git a/src/test/java/com/codeborne/selenide/impl/NavigatorTest.java b/src/test/java/com/codeborne/selenide/impl/NavigatorTest.java index 085e16914e..bd52b2c295 100644 --- a/src/test/java/com/codeborne/selenide/impl/NavigatorTest.java +++ b/src/test/java/com/codeborne/selenide/impl/NavigatorTest.java @@ -98,7 +98,7 @@ void open_addsEventToLog() { @Test void open_withoutAuthentication_resetsPreviousAuthentication() { Configuration.browser = "opera"; - enableProxy(); + Configuration.proxyEnabled = true; navigator.open("https://some.com/login"); @@ -109,7 +109,7 @@ void open_withoutAuthentication_resetsPreviousAuthentication() { @Test void open_withBasicAuth_noProxy() { Configuration.browser = "opera"; - disableProxy(); + Configuration.proxyEnabled = false; navigator.open("https://some.com/login", "", "basic-auth-login", "basic-auth-password"); @@ -119,7 +119,7 @@ void open_withBasicAuth_noProxy() { @Test void open_withBasicAuth_withProxy() { Configuration.browser = "opera"; - enableProxy(); + Configuration.proxyEnabled = true; navigator.open("https://some.com/login", "", "basic-auth-login", "basic-auth-password"); @@ -127,12 +127,4 @@ void open_withBasicAuth_withProxy() { verify(authenticationFilter) .setAuthentication(eq(AuthenticationType.BASIC), refEq(new Credentials("basic-auth-login", "basic-auth-password"))); } - - private void enableProxy() { - Configuration.fileDownload = Configuration.FileDownloadMode.PROXY; - } - - private void disableProxy() { - Configuration.fileDownload = Configuration.FileDownloadMode.HTTPGET; - } } diff --git a/src/test/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainerTest.java b/src/test/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainerTest.java index baf34a2e30..51a532d6d9 100644 --- a/src/test/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainerTest.java +++ b/src/test/java/com/codeborne/selenide/impl/WebDriverThreadLocalContainerTest.java @@ -1,11 +1,5 @@ package com.codeborne.selenide.impl; -import java.io.ByteArrayOutputStream; -import java.io.OutputStream; -import java.util.logging.Handler; -import java.util.logging.Logger; -import java.util.logging.StreamHandler; - import com.codeborne.selenide.Configuration; import com.codeborne.selenide.WebDriverRunner; import com.codeborne.selenide.webdriver.WebDriverFactory; @@ -13,7 +7,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.NoSuchWindowException; @@ -22,7 +15,12 @@ import org.openqa.selenium.chrome.ChromeDriver; import org.openqa.selenium.remote.UnreachableBrowserException; -import static com.codeborne.selenide.Configuration.FileDownloadMode.HTTPGET; +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; +import java.util.logging.Handler; +import java.util.logging.Logger; +import java.util.logging.StreamHandler; + import static com.codeborne.selenide.Configuration.FileDownloadMode.PROXY; import static com.codeborne.selenide.Selenide.close; import static java.lang.Thread.currentThread; @@ -67,7 +65,7 @@ void tearDown() { @Test void createWebDriverWithoutProxy() { - Configuration.fileDownload = HTTPGET; + Configuration.proxyEnabled = false; container.createDriver(); @@ -76,16 +74,24 @@ void createWebDriverWithoutProxy() { @Test void createWebDriverWithSelenideProxyServer() { + Configuration.proxyEnabled = true; + + container.createDriver(); + + assertThat(container.getProxyServer()).isNotNull(); + verify(container.factory).createWebDriver(container.getProxyServer().createSeleniumProxy()); + } + + @Test + void startsProxyServer_evenIfProxyIsNotEnabled_butFileDownloadModeIsProxy() { + Configuration.proxyEnabled = false; Configuration.fileDownload = PROXY; container.createDriver(); - ArgumentCaptor captor = ArgumentCaptor.forClass(Proxy.class); - verify(container.factory).createWebDriver(captor.capture()); - assertThat(captor.getValue().getHttpProxy()) - .isNotNull(); - assertThat(captor.getValue().getSslProxy()) - .isNotNull(); + assertThat(Configuration.proxyEnabled).isTrue(); + assertThat(container.getProxyServer()).isNotNull(); + verify(container.factory).createWebDriver(container.getProxyServer().createSeleniumProxy()); } @Test @@ -94,8 +100,7 @@ void checksIfBrowserIsStillAlive() { WebDriver webdriver = mock(WebDriver.class); container.THREAD_WEB_DRIVER.put(currentThread().getId(), webdriver); - assertThat(container.getAndCheckWebDriver()) - .isEqualTo(webdriver); + assertThat(container.getAndCheckWebDriver()).isEqualTo(webdriver); verify(container).isBrowserStillOpen(any()); } @@ -105,8 +110,7 @@ void doesNotReopenBrowserIfItFailed() { WebDriver webdriver = mock(WebDriver.class); container.THREAD_WEB_DRIVER.put(currentThread().getId(), webdriver); - assertThat(container.getAndCheckWebDriver()) - .isEqualTo(webdriver); + assertThat(container.getAndCheckWebDriver()).isEqualTo(webdriver); verify(container, never()).isBrowserStillOpen(any()); } @@ -115,8 +119,7 @@ void checksIfBrowserIsStillAlive_byCallingGetTitle() { WebDriver webdriver = mock(WebDriver.class); doReturn("blah").when(webdriver).getTitle(); - assertThat(container.isBrowserStillOpen(webdriver)) - .isTrue(); + assertThat(container.isBrowserStillOpen(webdriver)).isTrue(); } @Test @@ -124,8 +127,7 @@ void isBrowserStillOpen_UnreachableBrowserException() { WebDriver webdriver = mock(WebDriver.class); doThrow(UnreachableBrowserException.class).when(webdriver).getTitle(); - assertThat(container.isBrowserStillOpen(webdriver)) - .isFalse(); + assertThat(container.isBrowserStillOpen(webdriver)).isFalse(); } @Test @@ -133,8 +135,7 @@ void isBrowserStillOpen_NoSuchWindowException() { WebDriver webdriver = mock(WebDriver.class); doThrow(NoSuchWindowException.class).when(webdriver).getTitle(); - assertThat(container.isBrowserStillOpen(webdriver)) - .isFalse(); + assertThat(container.isBrowserStillOpen(webdriver)).isFalse(); } @Test @@ -142,14 +143,13 @@ void isBrowserStillOpen_NoSuchSessionException() { WebDriver webdriver = mock(WebDriver.class); doThrow(NoSuchSessionException.class).when(webdriver).getTitle(); - assertThat(container.isBrowserStillOpen(webdriver)) - .isFalse(); + assertThat(container.isBrowserStillOpen(webdriver)).isFalse(); } @Test void closeWebDriverLoggingWhenProxyIsAdded() { Configuration.holdBrowserOpen = false; - Configuration.fileDownload = PROXY; + Configuration.proxyEnabled = true; Proxy mockedProxy = Mockito.mock(Proxy.class); when(mockedProxy.getHttpProxy()).thenReturn("selenide:0"); @@ -181,7 +181,8 @@ void shouldNotOpenANewBrowser_ifSettingIsDisabled() { try { container.getWebDriver(); fail("expected IllegalStateException"); - } catch (IllegalStateException expected) { + } + catch (IllegalStateException expected) { assertThat(expected) .hasMessageContaining("reopenBrowserOnFail=false"); } diff --git a/src/test/java/com/codeborne/selenide/proxy/InetAddressResolverStub.java b/src/test/java/com/codeborne/selenide/proxy/InetAddressResolverStub.java new file mode 100644 index 0000000000..12145a71fa --- /dev/null +++ b/src/test/java/com/codeborne/selenide/proxy/InetAddressResolverStub.java @@ -0,0 +1,16 @@ +package com.codeborne.selenide.proxy; + +import java.net.InetAddress; +import java.net.UnknownHostException; + +class InetAddressResolverStub extends InetAddressResolver { + @Override + InetAddress getInetAddressByName(String hostname) { + try { + return InetAddress.getByAddress(hostname, new byte[4]); + } + catch (UnknownHostException e) { + throw new RuntimeException(e); + } + } +} diff --git a/src/test/java/com/codeborne/selenide/proxy/SelenideProxyServerTest.java b/src/test/java/com/codeborne/selenide/proxy/SelenideProxyServerTest.java index a589fb1626..859acf043e 100644 --- a/src/test/java/com/codeborne/selenide/proxy/SelenideProxyServerTest.java +++ b/src/test/java/com/codeborne/selenide/proxy/SelenideProxyServerTest.java @@ -1,12 +1,15 @@ package com.codeborne.selenide.proxy; -import java.net.InetSocketAddress; - +import com.codeborne.selenide.Configuration; import net.lightbody.bmp.BrowserMobProxyServer; import org.assertj.core.api.WithAssertions; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openqa.selenium.Proxy; +import java.net.InetSocketAddress; + import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -14,29 +17,46 @@ import static org.mockito.Mockito.when; class SelenideProxyServerTest implements WithAssertions { + private BrowserMobProxyServer bmp = mock(BrowserMobProxyServer.class); + private SelenideProxyServer proxyServer = new SelenideProxyServer(null, new InetAddressResolverStub(), bmp); + + @BeforeEach + @AfterEach + void resetProxySettings() { + Configuration.proxyPort = 0; + Configuration.proxyHost = ""; + } + @Test void canInterceptResponses() { - BrowserMobProxyServer bmp = mock(BrowserMobProxyServer.class); - when(bmp.getPort()).thenReturn(8888); - - SelenideProxyServer proxyServer = new SelenideProxyServer(null); - proxyServer.proxy = bmp; proxyServer.start(); - try { - verify(bmp).setTrustAllServers(true); - verify(bmp, never()).setChainedProxy(any(InetSocketAddress.class)); - verify(bmp).start(); - assertThat(proxyServer.createSeleniumProxy().getHttpProxy()) - .endsWith(":8888"); - } finally { - proxyServer.shutdown(); - } - verify(bmp).abort(); + verify(bmp).setTrustAllServers(true); + verify(bmp, never()).setChainedProxy(any(InetSocketAddress.class)); + verify(bmp).start(0); FileDownloadFilter filter = proxyServer.responseFilter("download"); - assertThat(filter.getDownloadedFiles().size()) - .isEqualTo(0); + assertThat(filter.getDownloadedFiles()).hasSize(0); + } + + @Test + void canShutdownProxyServer() { + proxyServer.shutdown(); + verify(bmp).abort(); + } + + @Test + void createSeleniumProxy() { + when(bmp.getPort()).thenReturn(8888); + + assertThat(proxyServer.createSeleniumProxy().getHttpProxy()).endsWith(":8888"); + } + + @Test + void createSeleniumProxy_withConfiguredHostname() { + Configuration.proxyHost = "my.megahost"; + when(bmp.getPort()).thenReturn(9999); + assertThat(proxyServer.createSeleniumProxy().getHttpProxy()).isEqualTo("my.megahost:9999"); } @Test @@ -46,9 +66,16 @@ void extractsProxyAddress() { InetSocketAddress proxyAddress = SelenideProxyServer.getProxyAddress(proxy); - assertThat(proxyAddress.getHostName()) - .isEqualTo("111.22.3.4444"); - assertThat(proxyAddress.getPort()) - .isEqualTo(8080); + assertThat(proxyAddress.getHostName()).isEqualTo("111.22.3.4444"); + assertThat(proxyAddress.getPort()).isEqualTo(8080); + } + + @Test + void canStartProxyServerOnConfiguredPort() { + Configuration.proxyPort = 666; + + proxyServer.start(); + + verify(bmp).start(666); } } diff --git a/src/test/java/grid/SeleniumGridTest.java b/src/test/java/grid/SeleniumGridTest.java index 70cfa7160a..3195eeee9e 100644 --- a/src/test/java/grid/SeleniumGridTest.java +++ b/src/test/java/grid/SeleniumGridTest.java @@ -14,7 +14,6 @@ import java.util.Optional; import static com.codeborne.selenide.CollectionCondition.size; -import static com.codeborne.selenide.Configuration.FileDownloadMode.PROXY; import static com.codeborne.selenide.Selenide.$$; import static com.codeborne.selenide.Selenide.close; import static com.codeborne.selenide.WebDriverRunner.getWebDriver; @@ -39,7 +38,7 @@ void setUp() { Configuration.remote = "http://localhost:" + hubPort + "/wd/hub"; Configuration.browser = "chrome"; Configuration.headless = true; - Configuration.fileDownload = PROXY; + Configuration.proxyEnabled = true; new WebDriverBinaryManager().setupBinaryPath(); } diff --git a/src/test/java/integration/IntegrationTest.java b/src/test/java/integration/IntegrationTest.java index ac75579955..95dabf0a27 100644 --- a/src/test/java/integration/IntegrationTest.java +++ b/src/test/java/integration/IntegrationTest.java @@ -99,6 +99,8 @@ private void resetSettings() { versatileSetValue = false; browserSize = "1200x960"; server.reset(); + Configuration.proxyPort = 0; + Configuration.proxyHost = ""; // proxy breaks Firefox/Marionette because of this error: // "InvalidArgumentError: Expected [object Undefined] undefined to be an integer" From 16451231605b8c9b88c481dd06ac35a57643347a Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Tue, 28 Aug 2018 17:24:39 +0300 Subject: [PATCH 2/2] #788 remove unneeded else blocks --- .../com/codeborne/selenide/commands/DownloadFile.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/codeborne/selenide/commands/DownloadFile.java b/src/main/java/com/codeborne/selenide/commands/DownloadFile.java index 06646b857b..f4defe71e8 100644 --- a/src/main/java/com/codeborne/selenide/commands/DownloadFile.java +++ b/src/main/java/com/codeborne/selenide/commands/DownloadFile.java @@ -40,15 +40,14 @@ public File execute(SelenideElement proxy, WebElementSource linkWithHref, Object LOG.config("selenide.fileDownload = " + System.getProperty("selenide.fileDownload") + " download file via http get"); return downloadFileWithHttpRequest.download(link, timeout); } - else if (!Configuration.proxyEnabled) { + if (!Configuration.proxyEnabled) { throw new IllegalStateException("Cannot download file: proxy server is not enabled. Setup Configuration.proxyEnabled"); } - else if (webdriverContainer.getProxyServer() == null) { + if (webdriverContainer.getProxyServer() == null) { throw new IllegalStateException("Cannot download file: proxy server is not started"); } - else { - return downloadFileWithProxyServer.download(linkWithHref, link, webdriverContainer.getProxyServer(), timeout); - } + + return downloadFileWithProxyServer.download(linkWithHref, link, webdriverContainer.getProxyServer(), timeout); } long getTimeout(Object[] args) {