Skip to content

Commit

Permalink
Fix flakiness in LoggerTest (#885)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
natemort committed Apr 25, 2024
1 parent bfd521d commit ec10e2b
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions src/test/java/com/uber/cadence/workflow/LoggerTest.java
Expand Up @@ -22,7 +22,7 @@

import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import ch.qos.logback.core.AppenderBase;
import com.uber.cadence.client.WorkflowClient;
import com.uber.cadence.client.WorkflowClientOptions;
import com.uber.cadence.client.WorkflowOptions;
Expand All @@ -31,14 +31,16 @@
import com.uber.cadence.testing.TestWorkflowEnvironment;
import com.uber.cadence.worker.Worker;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class LoggerTest {

private static final ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
private static final ThreadSafeListAppender listAppender = new ThreadSafeListAppender();

static {
LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
Expand Down Expand Up @@ -118,7 +120,7 @@ public void testWorkflowLogger() {

private int matchingLines(String message) {
int i = 0;
for (ILoggingEvent event : listAppender.list) {
for (ILoggingEvent event : listAppender.getEvents()) {
if (event.getFormattedMessage().contains(message)) {
assertTrue(event.getMDCPropertyMap().containsKey(LoggerTag.WORKFLOW_ID));
assertTrue(event.getMDCPropertyMap().containsKey(LoggerTag.WORKFLOW_TYPE));
Expand All @@ -129,4 +131,18 @@ private int matchingLines(String message) {
}
return i;
}

private static class ThreadSafeListAppender extends AppenderBase<ILoggingEvent> {

private final List<ILoggingEvent> events = new ArrayList<>();

@Override
protected synchronized void append(ILoggingEvent event) {
events.add(event);
}

public synchronized List<ILoggingEvent> getEvents() {
return new ArrayList<>(events);
}
}
}

0 comments on commit ec10e2b

Please sign in to comment.