Resolve carry-over state issue in TestServerRule

The TestServerRule object is shared between retries in the same test,
so the testServer object constructed in TestServerRule's constructor
will be shared as well. This should be the reason why the test retry
carries over some state. (The log in the test proves that the
testServer object is shared because it listens to the same port in
all retries, which should not happen if its constructor is invoked
every time. You can find multiple "TestServerRule is listening on:
[]in this test
[]

So, this CL delayed the construction to rule.before() method which is
invoked before every retry.(You can see each retry has a test server
listening to different port and the error is "It differed by <16> pixels."
for all attempts instead of not clickable button []

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=246904446
This commit is contained in:
shicong 2019-05-06 15:03:42 -07:00 committed by jianglai
parent d1b7824cc2
commit e382299212
2 changed files with 25 additions and 12 deletions

View file

@ -153,7 +153,7 @@ public class RepeatableRunner extends BlockJUnit4ClassRunner {
} catch (Throwable e) {
numFailure++;
lastException = e;
logger.atWarning().log(
logger.atWarning().withCause(e).log(
"[%s] Attempt %d of %d failed!\n", method.getName(), attempt, MAX_ATTEMPTS);
}
}

View file

@ -54,8 +54,11 @@ public final class TestServerRule extends ExternalResource {
private final ImmutableList<Fixture> fixtures;
private final AppEngineRule appEngineRule;
private final BlockingQueue<FutureTask<?>> jobs = new LinkedBlockingDeque<>();
private final TestServer testServer;
private final ImmutableMap<String, Path> runfiles;
private final ImmutableList<Route> routes;
private final ImmutableList<Class<? extends Filter>> filters;
private TestServer testServer;
private Thread serverThread;
private TestServerRule(
@ -64,6 +67,9 @@ public final class TestServerRule extends ExternalResource {
ImmutableList<Class<? extends Filter>> filters,
ImmutableList<Fixture> fixtures,
String email) {
this.runfiles = runfiles;
this.routes = routes;
this.filters = filters;
this.fixtures = fixtures;
// We create an GAE-Admin user, and then use AuthenticatedRegistrarAccessor.bypassAdminCheck to
// choose whether the user is an admin or not.
@ -74,8 +80,12 @@ public final class TestServerRule extends ExternalResource {
.withTaskQueue()
.withUserService(UserInfo.createAdmin(email, THE_REGISTRAR_GAE_USER_ID))
.build();
}
@Override
protected void before() throws InterruptedException {
try {
this.testServer =
testServer =
new TestServer(
HostAndPort.fromParts(
// Use external IP address here so the browser running inside Docker container
@ -87,15 +97,14 @@ public final class TestServerRule extends ExternalResource {
} catch (UnknownHostException e) {
throw new IllegalStateException(e);
}
}
@Override
protected void before() throws InterruptedException {
setIsAdmin(false);
serverThread = new Thread(new Server());
synchronized (testServer) {
Server server = new Server();
serverThread = new Thread(server);
synchronized (this) {
serverThread.start();
testServer.wait();
while (!server.isRunning) {
this.wait();
}
}
}
@ -112,6 +121,7 @@ public final class TestServerRule extends ExternalResource {
} finally {
serverThread = null;
jobs.clear();
testServer = null;
}
}
@ -149,6 +159,8 @@ public final class TestServerRule extends ExternalResource {
}
private final class Server extends Statement implements Runnable {
private volatile boolean isRunning = false;
@Override
public void run() {
try {
@ -169,8 +181,9 @@ public final class TestServerRule extends ExternalResource {
}
testServer.start();
System.out.printf("TestServerRule is listening on: %s\n", testServer.getUrl("/"));
synchronized (testServer) {
testServer.notify();
synchronized (TestServerRule.this) {
isRunning = true;
TestServerRule.this.notify();
}
try {
while (true) {