Skip to content

Commit ec10e2b

Browse files
authored
Fix flakiness in LoggerTest (#885)
If any other threads logs a message while we iterate over the events it can throw a ConcurrentModificationException. Create our own implementation of ListAppender that uses the synchronized keyword to protect reading/writing to the backing list with a mutex. Create a copy of the list while holding to the lock to support iterating over it.
1 parent bfd521d commit ec10e2b

File tree

1 file changed

+19
-3
lines changed

1 file changed

+19
-3
lines changed

src/test/java/com/uber/cadence/workflow/LoggerTest.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
import ch.qos.logback.classic.LoggerContext;
2424
import ch.qos.logback.classic.spi.ILoggingEvent;
25-
import ch.qos.logback.core.read.ListAppender;
25+
import ch.qos.logback.core.AppenderBase;
2626
import com.uber.cadence.client.WorkflowClient;
2727
import com.uber.cadence.client.WorkflowClientOptions;
2828
import com.uber.cadence.client.WorkflowOptions;
@@ -31,14 +31,16 @@
3131
import com.uber.cadence.testing.TestWorkflowEnvironment;
3232
import com.uber.cadence.worker.Worker;
3333
import java.time.Duration;
34+
import java.util.ArrayList;
35+
import java.util.List;
3436
import java.util.UUID;
3537
import org.junit.Test;
3638
import org.slf4j.Logger;
3739
import org.slf4j.LoggerFactory;
3840

3941
public class LoggerTest {
4042

41-
private static final ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
43+
private static final ThreadSafeListAppender listAppender = new ThreadSafeListAppender();
4244

4345
static {
4446
LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
@@ -118,7 +120,7 @@ public void testWorkflowLogger() {
118120

119121
private int matchingLines(String message) {
120122
int i = 0;
121-
for (ILoggingEvent event : listAppender.list) {
123+
for (ILoggingEvent event : listAppender.getEvents()) {
122124
if (event.getFormattedMessage().contains(message)) {
123125
assertTrue(event.getMDCPropertyMap().containsKey(LoggerTag.WORKFLOW_ID));
124126
assertTrue(event.getMDCPropertyMap().containsKey(LoggerTag.WORKFLOW_TYPE));
@@ -129,4 +131,18 @@ private int matchingLines(String message) {
129131
}
130132
return i;
131133
}
134+
135+
private static class ThreadSafeListAppender extends AppenderBase<ILoggingEvent> {
136+
137+
private final List<ILoggingEvent> events = new ArrayList<>();
138+
139+
@Override
140+
protected synchronized void append(ILoggingEvent event) {
141+
events.add(event);
142+
}
143+
144+
public synchronized List<ILoggingEvent> getEvents() {
145+
return new ArrayList<>(events);
146+
}
147+
}
132148
}

0 commit comments

Comments
 (0)