Skip to content

Commit 8687bb0

Browse files
kweinmeisterkweinmeister
authored
fix: flaky tictactoe test (GoogleCloudPlatform#8347)
1 parent 75c6188 commit 8687bb0

2 files changed

Lines changed: 18 additions & 50 deletions

File tree

‎appengine-java8/firebase-tictactoe/src/main/java/com/example/appengine/firetactoe/Game.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@
1616

1717
package com.example.appengine.firetactoe;
1818

19+
import com.google.cloud.Timestamp;
1920
import com.googlecode.objectify.annotation.Entity;
2021
import com.googlecode.objectify.annotation.Id;
22+
import com.googlecode.objectify.annotation.Index;
2123
import java.io.IOException;
2224
import java.util.UUID;
2325
import java.util.logging.Level;
@@ -57,6 +59,8 @@ public class Game {
5759

5860
@Id
5961
public String id;
62+
@Index
63+
public Timestamp created;
6064
public String userX;
6165
public String userO;
6266
public String board;
@@ -67,11 +71,12 @@ public class Game {
6771
private static final Logger LOGGER = Logger.getLogger(Game.class.getName());
6872

6973
Game() {
70-
this.id = UUID.randomUUID().toString();
74+
this(null, null, null, false);
7175
}
7276

7377
Game(String userX, String userO, String board, boolean moveX) {
7478
this.id = UUID.randomUUID().toString();
79+
this.created = Timestamp.now();
7580
this.userX = userX;
7681
this.userO = userO;
7782
this.board = board;

‎appengine-java8/firebase-tictactoe/src/test/java/com/example/appengine/firetactoe/TicTacToeServletTest.java‎

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020
import static org.mockito.Mockito.anyString;
21-
import static org.mockito.Mockito.atLeastOnce;
2221
import static org.mockito.Mockito.eq;
2322
import static org.mockito.Mockito.spy;
2423
import static org.mockito.Mockito.times;
@@ -34,7 +33,6 @@
3433
import com.google.appengine.tools.development.testing.LocalServiceTestHelper;
3534
import com.google.appengine.tools.development.testing.LocalURLFetchServiceTestConfig;
3635
import com.google.appengine.tools.development.testing.LocalUserServiceTestConfig;
37-
import com.google.cloud.datastore.QueryResults;
3836
import com.google.cloud.testing.junit4.MultipleAttemptsRule;
3937
import com.google.common.collect.ImmutableMap;
4038
import com.googlecode.objectify.Objectify;
@@ -43,9 +41,6 @@
4341
import java.io.ByteArrayInputStream;
4442
import java.io.IOException;
4543
import java.util.HashMap;
46-
import java.util.HashSet;
47-
import java.util.Set;
48-
import java.util.concurrent.ThreadLocalRandom;
4944
import javax.servlet.RequestDispatcher;
5045
import javax.servlet.http.HttpServletRequest;
5146
import javax.servlet.http.HttpServletResponse;
@@ -77,8 +72,6 @@ public class TicTacToeServletTest {
7772
.setDefaultHighRepJobPolicyUnappliedJobPercentage(0),
7873
new LocalUserServiceTestConfig(),
7974
new LocalURLFetchServiceTestConfig())
80-
.setEnvInstance(
81-
String.valueOf(ThreadLocalRandom.current().nextInt(0, Integer.MAX_VALUE)))
8275
.setEnvEmail(USER_EMAIL)
8376
.setEnvAuthDomain("gmail.com")
8477
.setEnvAttributes(new HashMap<>(ImmutableMap
@@ -110,7 +103,6 @@ public void setUp() throws Exception {
110103
// Set up a fake HTTP response.
111104
when(mockRequest.getRequestURL()).thenReturn(new StringBuffer("https://timbre/"));
112105
when(mockRequest.getRequestDispatcher("/WEB-INF/view/index.jsp")).thenReturn(requestDispatcher);
113-
114106
servletUnderTest = new TicTacToeServlet();
115107

116108
helper.setEnvIsLoggedIn(true);
@@ -122,29 +114,6 @@ public void tearDown() {
122114
helper.tearDown();
123115
}
124116

125-
/**
126-
* Compares game results before and after to find new game. Objectify v6 does
127-
* not guarantee new record is first.
128-
*
129-
* @param before query containing games before inserting
130-
* @param after query containing games after inserting
131-
* @return the game that was not in the original query
132-
*/
133-
private Game getNewGame(QueryResults<Game> before, QueryResults<Game> after) {
134-
Set<String> gameKeys = new HashSet<>();
135-
while (before.hasNext()) {
136-
String gameKey = before.next().id;
137-
gameKeys.add(gameKey);
138-
}
139-
while (after.hasNext()) {
140-
Game game = after.next();
141-
if (!gameKeys.contains(game.id)) {
142-
return game;
143-
}
144-
}
145-
return null;
146-
}
147-
148117
@Test
149118
public void doGetNoGameKey() throws Exception {
150119
// Mock out the firebase response. See
@@ -167,29 +136,23 @@ public LowLevelHttpResponse execute() throws IOException {
167136
});
168137
FirebaseChannel.getInstance().httpTransport = mockHttpTransport;
169138

139+
servletUnderTest.doGet(mockRequest, mockResponse);
140+
170141
// Make sure the game object was created for a new game
171142
Objectify ofy = ObjectifyService.ofy();
172-
Game game = null;
173-
int count = 0;
174-
while (game == null && count < 5) {
175-
QueryResults<Game> before = ofy.load().type(Game.class).iterator();
176-
servletUnderTest.doGet(mockRequest, mockResponse);
177-
QueryResults<Game> after = ofy.load().type(Game.class).iterator();
178-
game = getNewGame(before, after);
179-
count++;
180-
}
181-
assertThat(game).isNotNull();
143+
// Get the game with the most recent create date
144+
Game game = ofy.load().type(Game.class).order("-created").first().safe();
182145
assertThat(game.userX).isEqualTo(USER_ID);
183146

184-
verify(mockHttpTransport, atLeastOnce()).buildRequest(eq("PATCH"),
147+
verify(mockHttpTransport).buildRequest(eq("PATCH"),
185148
ArgumentMatchers.matches(FIREBASE_DB_URL + "/channels/[\\w-]+.json$"));
186-
verify(requestDispatcher, atLeastOnce()).forward(mockRequest, mockResponse);
187-
verify(mockRequest, atLeastOnce()).setAttribute(eq("token"), anyString());
188-
verify(mockRequest, atLeastOnce()).setAttribute("game_key", game.id);
189-
verify(mockRequest, atLeastOnce()).setAttribute("me", USER_ID);
190-
verify(mockRequest, atLeastOnce()).setAttribute("channel_id", USER_ID + game.id);
191-
verify(mockRequest, atLeastOnce()).setAttribute(eq("initial_message"), anyString());
192-
verify(mockRequest, atLeastOnce()).setAttribute(eq("game_link"), anyString());
149+
verify(requestDispatcher).forward(mockRequest, mockResponse);
150+
verify(mockRequest).setAttribute(eq("token"), anyString());
151+
verify(mockRequest).setAttribute("game_key", game.id);
152+
verify(mockRequest).setAttribute("me", USER_ID);
153+
verify(mockRequest).setAttribute("channel_id", USER_ID + game.id);
154+
verify(mockRequest).setAttribute(eq("initial_message"), anyString());
155+
verify(mockRequest).setAttribute(eq("game_link"), anyString());
193156
}
194157

195158
@Test

0 commit comments

Comments
 (0)