Skip to content

Fix Guava compatibility issues and flaky tests #648

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Fix Guava compatibility issues and flaky tests #648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0010cc2
Replace usage of deprecated c.g.c.n.HostAndPort.getHostText with Host…
valfirst Jun 7, 2017
86d1e42
Fix flaky tests: use nanoseconds instead of milliseconds
valfirst Jun 7, 2017
494c491
Use login.newrelic.com because rpm.newrelic.com requires authenticati…
valfirst Jun 7, 2017
91c163d
Replace msn.com with msn.com/en-us/ to avoid redirection
valfirst Jun 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion browsermob-core/src/main/java/net/lightbody/bmp/filters/HttpsAwareFiltersAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public String getHost(HttpRequest modifiedRequest) {
String serverHost;
if (isHttps()) {
HostAndPort hostAndPort = HostAndPort.fromString(getHttpsRequestHostAndPort());
serverHost = hostAndPort.getHostText();
serverHost = hostAndPort.getHost();
} else {
serverHost = HttpUtil.getHostFromRequest(modifiedRequest);
}
Expand Down
2 changes: 1 addition & 1 deletion browsermob-core/src/main/java/net/lightbody/bmp/filters/ResolvedHostnameCacheFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void proxyToServerResolutionSucceeded(String serverHostAndPort, InetSocke
if (resolvedAddress != null) {
// place the resolved host into the hostname cache, so subsequent requests will be able to identify the IP address
HostAndPort parsedHostAndPort = HostAndPort.fromString(serverHostAndPort);
String host = parsedHostAndPort.getHostText();
String host = parsedHostAndPort.getHost();

if (host != null && !host.isEmpty()) {
resolvedAddresses.put(host, resolvedAddress.getHostAddress());
Expand Down
2 changes: 1 addition & 1 deletion browsermob-core/src/main/java/net/lightbody/bmp/util/BrowserMobHttpUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ public static String removeMatchingPort(String hostWithPort, int portNumber) {
HostAndPort parsedHostAndPort = HostAndPort.fromString(hostWithPort);
if (parsedHostAndPort.hasPort() && parsedHostAndPort.getPort() == portNumber) {
// HostAndPort.getHostText() strips brackets from ipv6 addresses, so reparse using fromHost
return HostAndPort.fromHost(parsedHostAndPort.getHostText()).toString();
return HostAndPort.fromHost(parsedHostAndPort.getHost()).toString();
} else {
return hostWithPort;
}
Expand Down
34 changes: 17 additions & 17 deletions browsermob-core/src/test/java/net/lightbody/bmp/proxy/dns/AdvancedHostResolverCacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void testCanClearDNSCache() {
resolver.resolve("www.msn.com");
long finish = System.nanoTime();

assertNotEquals("Expected non-zero DNS lookup time for www.msn.com after clearing DNS cache", 0, TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS));
assertNotEquals("Expected non-zero DNS lookup time for www.msn.com after clearing DNS cache", 0, finish - start);
}

@Test
Expand All @@ -81,17 +81,17 @@ public void testCachedPositiveLookup() {
resolver.resolve("news.bing.com");
long finish = System.nanoTime();

long uncachedLookupMs = TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS);
long uncachedLookupNs = finish - start;

assertNotEquals("Expected non-zero DNS lookup time for news.bing.com on first lookup", 0, uncachedLookupMs);
assertNotEquals("Expected non-zero DNS lookup time for news.bing.com on first lookup", 0, uncachedLookupNs);

start = System.nanoTime();
resolver.resolve("news.bing.com");
finish = System.nanoTime();

long cachedLookupMs = TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS);
long cachedLookupNs = finish - start;

assertTrue("Expected extremely fast DNS lookup time for news.bing.com on second (cached) lookup. Uncached: " + uncachedLookupMs + "ms; cached: " + cachedLookupMs + "ms.", cachedLookupMs <= uncachedLookupMs / 2);
assertTrue("Expected extremely fast DNS lookup time for news.bing.com on second (cached) lookup. Uncached: " + uncachedLookupNs + "ns; cached: " + cachedLookupNs + "ns.", cachedLookupNs <= uncachedLookupNs / 2);
}

@Test
Expand All @@ -100,17 +100,17 @@ public void testCachedNegativeLookup() {
resolver.resolve("fake.notarealaddress");
long finish = System.nanoTime();

long uncachedLookupMs = TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS);
long uncachedLookupNs = finish - start;

assertNotEquals("Expected non-zero DNS lookup time for fake.notarealaddress on first lookup", 0, uncachedLookupMs);
assertNotEquals("Expected non-zero DNS lookup time for fake.notarealaddress on first lookup", 0, uncachedLookupNs);

start = System.nanoTime();
resolver.resolve("fake.notarealaddress");
finish = System.nanoTime();

long cachedLookupMs = TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS);
long cachedLookupNs = finish - start;

assertTrue("Expected extremely fast DNS lookup time for fake.notarealaddress on second (cached) lookup. Uncached: " + uncachedLookupMs + "ms; cached: " + cachedLookupMs + "ms.", cachedLookupMs <= uncachedLookupMs / 2);
assertTrue("Expected extremely fast DNS lookup time for fake.notarealaddress on second (cached) lookup. Uncached: " + uncachedLookupNs + "ns; cached: " + cachedLookupNs + "ns.", cachedLookupNs <= uncachedLookupNs / 2);
}

@Test
Expand All @@ -135,7 +135,7 @@ public void testSetPositiveCacheTtl() throws InterruptedException {
assertNotNull("Collection of resolved addresses should never be null", addresses);
assertNotEquals("Expected to find addresses for www.msn.com", 0, addresses.size());

assertNotEquals("Expected non-zero DNS lookup time for www.msn.com after setting positive cache TTL", 0, TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS));
assertNotEquals("Expected non-zero DNS lookup time for www.msn.com after setting positive cache TTL", 0, finish - start);
}

@Test
Expand Down Expand Up @@ -163,7 +163,7 @@ public void testSetNegativeCacheTtl() throws InterruptedException {
assertNotNull("Collection of resolved addresses should never be null", addresses);
assertEquals("Expected to find no addresses for " + fakeAddress, 0, addresses.size());

assertNotEquals("Expected non-zero DNS lookup time for " + fakeAddress + " after setting negative cache TTL", 0, TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS));
assertNotEquals("Expected non-zero DNS lookup time for " + fakeAddress + " after setting negative cache TTL", 0, finish - start);
}

@Test
Expand All @@ -185,12 +185,12 @@ public void testSetEternalNegativeCacheTtl() {
addresses = resolver.resolve(fakeAddress);
long finish = System.nanoTime();

long cachedLookupMs = TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS);
long cachedLookupNs = finish - start;

assertNotNull("Collection of resolved addresses should never be null", addresses);
assertEquals("Expected to find no addresses for " + fakeAddress, 0, addresses.size());

assertTrue("Expected extremely fast DNS lookup time for " + fakeAddress + " after setting eternal negative cache TTL. Cached lookup time: " + cachedLookupMs + "ms.", cachedLookupMs <= 10);
assertTrue("Expected extremely fast DNS lookup time for " + fakeAddress + " after setting eternal negative cache TTL. Cached lookup time: " + cachedLookupNs + "ns.", cachedLookupNs <= TimeUnit.NANOSECONDS.convert(10, TimeUnit.MILLISECONDS));
}

@Test
Expand All @@ -204,7 +204,7 @@ public void testSetEternalPositiveCacheTtl() {
long one = System.nanoTime();
Collection<InetAddress> addresses = resolver.resolve("www.msn.com");
long two = System.nanoTime();
log.info("Time to resolve address without cache: {}ms", TimeUnit.MILLISECONDS.convert(two - one, TimeUnit.NANOSECONDS));
log.info("Time to resolve address without cache: {}ns", two - one);

// make sure there are addresses, since this is a *positive* TTL test
assertNotNull("Collection of resolved addresses should never be null", addresses);
Expand All @@ -214,13 +214,13 @@ public void testSetEternalPositiveCacheTtl() {
addresses = resolver.resolve("www.msn.com");
long finish = System.nanoTime();

long cachedLookupMs = TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS);
long cachedLookupNs = finish - start;

log.info("Time to resolve address with cache: {}ms", TimeUnit.MILLISECONDS.convert(finish - start, TimeUnit.NANOSECONDS));
log.info("Time to resolve address with cache: {}ns", cachedLookupNs);

assertNotNull("Collection of resolved addresses should never be null", addresses);
assertNotEquals("Expected to find addresses for www.msn.com", 0, addresses.size());

assertTrue("Expected extremely fast DNS lookup time for www.msn.com after setting eternal negative cache TTL. Cached lookup time: " + cachedLookupMs + "ms.", cachedLookupMs <= 10);
assertTrue("Expected extremely fast DNS lookup time for www.msn.com after setting eternal negative cache TTL. Cached lookup time: " + cachedLookupNs + "ns.", cachedLookupNs <= TimeUnit.NANOSECONDS.convert(10, TimeUnit.MILLISECONDS));
}
}
2 changes: 1 addition & 1 deletion browsermob-legacy/src/main/java/net/lightbody/bmp/BrowserMobProxyServerLegacyAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ public void setOptions(Map<String, String> options) {
log.warn("Chained proxy support through setOptions is deprecated. Use setUpstreamProxy() to enable chained proxy support.");

HostAndPort hostAndPort = HostAndPort.fromString(httpProxy);
this.setChainedProxy(new InetSocketAddress(hostAndPort.getHostText(), hostAndPort.getPortOrDefault(80)));
this.setChainedProxy(new InetSocketAddress(hostAndPort.getHost(), hostAndPort.getPortOrDefault(80)));
} else {
if (errorOnUnsupportedOperation) {
throw new UnsupportedOperationException("The LittleProxy-based implementation of BrowserMob Proxy does not support the setOptions method. Use the methods defined in the BrowserMobProxy interface to set connection parameters.");
Expand Down
5 changes: 2 additions & 3 deletions browsermob-legacy/src/test/java/net/lightbody/bmp/proxy/HarTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import net.lightbody.bmp.core.har.HarEntry;
import net.lightbody.bmp.core.har.HarLog;
import net.lightbody.bmp.core.har.HarNameValuePair;
import net.lightbody.bmp.core.har.HarNameVersion;
import net.lightbody.bmp.core.har.HarPage;
import net.lightbody.bmp.core.har.HarPageTimings;
import net.lightbody.bmp.core.har.HarPostData;
Expand Down Expand Up @@ -444,7 +443,7 @@ public void testEntryFieldsPopulatedForHttp() throws IOException, InterruptedExc
proxy.newHar("testEntryFieldsPopulatedForHttp");

// not using localhost so we get >0ms timing
HttpGet get = new HttpGet("http://www.msn.com");
HttpGet get = new HttpGet("https://www.msn.com/en-us/");
IOUtils.toStringAndClose(client.execute(get).getEntity().getContent());

proxy.endPage();
Expand Down Expand Up @@ -501,7 +500,7 @@ public void testEntryFieldsPopulatedForHttps() throws IOException, InterruptedEx
proxy.newHar("testEntryFieldsPopulatedForHttps");

// not using localhost so we get >0ms timing
HttpGet get = new HttpGet("https://www.msn.com");
HttpGet get = new HttpGet("https://www.msn.com/en-us/");
IOUtils.toStringAndClose(client.execute(get).getEntity().getContent());

proxy.endPage();
Expand Down
6 changes: 3 additions & 3 deletions browsermob-legacy/src/test/java/net/lightbody/bmp/proxy/SslTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ public void testSalesforce() throws Exception {
@Test
public void testNewRelic() throws Exception {
// see https://github.com/webmetrics/browsermob-proxy/issues/105
proxy.remapHost("foo.newrelic.com", "rpm.newrelic.com");
proxy.remapHost("bar.newrelic.com", "rpm.newrelic.com");
proxy.remapHost("foo.newrelic.com", "login.newrelic.com");
proxy.remapHost("bar.newrelic.com", "login.newrelic.com");
get("https://foo.newrelic.com/");
get("https://bar.newrelic.com/");
get("https://rpm.newrelic.com/");
get("https://login.newrelic.com/");
}

private void get(String url) throws IOException {
Expand Down
2 changes: 1 addition & 1 deletion mitm/src/main/java/net/lightbody/bmp/util/HttpUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private static String parseHostHeader(HttpRequest httpRequest, boolean includePo
return hostAndPort;
} else {
HostAndPort parsedHostAndPort = HostAndPort.fromString(hostAndPort);
return parsedHostAndPort.getHostText();
return parsedHostAndPort.getHost();
}
} else {
return null;
Expand Down