Skip to content

Commit

Permalink
Make plugin location monitor tests faster and more robust
Browse files Browse the repository at this point in the history 
These fail sporadically quite often, because file system events are a bit unpredictable, and because
of race conditions in assertions. Simplify, speed up and improve these.
  • Loading branch information
@chadlwilson
chadlwilson committed Oct 10, 2023
1 parent b6f06f1 commit 457240d
Show file tree
Hide file tree
Showing 5 changed files with 267 additions and 238 deletions.
6 changes: 5 additions & 1 deletion base/src/main/java/com/thoughtworks/go/util/SystemEnvironment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ public String getCruiseWar() {
}

public long getUnresponsiveJobWarningThreshold() {
return Long.parseLong(getPropertyImpl(UNRESPONSIVE_JOB_WARNING_THRESHOLD, "5")) * 60 * 1000;//mins to mills
return Long.parseLong(getPropertyImpl(UNRESPONSIVE_JOB_WARNING_THRESHOLD, "5")) * 60 * 1000; // mins to millis
}

public boolean getParentLoaderPriority() {
Expand Down Expand Up @@ -696,6 +696,10 @@ public boolean shouldInitializeConfigRepositoriesOnStartup() {
return INITIALIZE_CONFIG_REPOSITORIES_ON_STARTUP.getValue();
}

public long getPluginLocationMonitorIntervalInMillis() {
return SECONDS.toMillis(PLUGIN_LOCATION_MONITOR_INTERVAL_IN_SECONDS.getValue());
}

public static abstract class GoSystemProperty<T> {
private final String propertyName;
protected T defaultValue;
Expand Down
28 changes: 13 additions & 15 deletions ...c/main/java/com/thoughtworks/go/plugin/infra/monitor/DefaultPluginJarLocationMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.thoughtworks.go.plugin.infra.monitor;

import com.thoughtworks.go.util.SystemEnvironment;
import org.apache.commons.collections4.Closure;
import org.apache.commons.collections4.IterableUtils;
import org.apache.commons.io.FileUtils;
import org.slf4j.Logger;
Expand All @@ -29,8 +28,10 @@
import java.lang.ref.WeakReference;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Consumer;
import java.util.stream.Collectors;

import static com.thoughtworks.go.util.SystemEnvironment.*;
Expand Down Expand Up @@ -81,14 +82,11 @@ public void start() {

@Override
public boolean hasRunAtLeastOnce() {
return getLastRun() > 0;
return hasRunSince(0);
}

long getLastRun() {
if (monitorThread == null) {
return 0;
}
return monitorThread.getLastRun();
public boolean hasRunSince(long timestamp) {
return Optional.ofNullable(monitorThread).map(t -> t.hasRunSince(timestamp)).orElse(false);
}

private void initializeMonitorThread() {
Expand Down Expand Up @@ -159,7 +157,7 @@ private static class PluginLocationMonitorThread extends Thread {
private final File externalPluginDirectory;
private final List<WeakReference<PluginJarChangeListener>> pluginJarChangeListener;
private final SystemEnvironment systemEnvironment;
private long lastRun;
private volatile long lastRun;

public PluginLocationMonitorThread(File bundledPluginDirectory,
File externalPluginDirectory,
Expand All @@ -178,7 +176,7 @@ public void run() {
do {
oneShot();

int interval = systemEnvironment.get(PLUGIN_LOCATION_MONITOR_INTERVAL_IN_SECONDS);
long interval = systemEnvironment.getPluginLocationMonitorIntervalInMillis();
if (interval <= 0) {
break;
}
Expand All @@ -195,8 +193,8 @@ public synchronized void oneShot() {
lastRun = System.currentTimeMillis();
}

synchronized long getLastRun() {
return lastRun;
boolean hasRunSince(long timestamp) {
return lastRun > timestamp;
}

private Set<BundleOrPluginFileDetails> loadAndNotifyPluginsFrom(File pluginDirectory,
Expand All @@ -211,9 +209,9 @@ private PluginJarChangeListener doOnAllListeners() {
return new DoOnAllListeners(pluginJarChangeListener);
}

private void waitForMonitorInterval(int intervalSeconds) {
private void waitForMonitorInterval(long intervalMillis) {
try {
Thread.sleep(intervalSeconds * 1000L);
Thread.sleep(intervalMillis);
} catch (InterruptedException e) {
this.interrupt();
}
Expand Down Expand Up @@ -249,15 +247,15 @@ public void pluginJarRemoved(final BundleOrPluginFileDetails bundleOrPluginFileD
doOnAllPluginJarChangeListener(o -> o.pluginJarRemoved(bundleOrPluginFileDetails));
}

private void doOnAllPluginJarChangeListener(Closure<PluginJarChangeListener> closure) {
private void doOnAllPluginJarChangeListener(Consumer<PluginJarChangeListener> closure) {
for (WeakReference<PluginJarChangeListener> listener : listeners) {
PluginJarChangeListener changeListener = listener.get();
if (changeListener == null) {
continue;
}

try {
closure.execute(changeListener);
closure.accept(changeListener);
} catch (Exception e) {
LOGGER.warn("Plugin listener failed", e);
}
Expand Down
66 changes: 31 additions & 35 deletions ...com/thoughtworks/go/plugin/infra/monitor/AbstractDefaultPluginJarLocationMonitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,64 +16,60 @@
package com.thoughtworks.go.plugin.infra.monitor;

import com.thoughtworks.go.plugin.FileHelper;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.Charset;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

import static org.mockito.Mockito.*;

@ExtendWith(MockitoExtension.class)
abstract class AbstractDefaultPluginJarLocationMonitorTest {
private static final int NO_OF_TRIES_TO_CHECK_MONITOR_RUN = 150;
public static final long MONITOR_WAIT_MILLIS = TimeUnit.SECONDS.toMillis(2);

File pluginWorkDir;
FileHelper tempFolder;

@Mock
PluginJarChangeListener changeListener;

@BeforeEach
void setUp(@TempDir File tempFolder) throws Exception {
this.tempFolder = new FileHelper(tempFolder);
pluginWorkDir = this.tempFolder.newFolder("plugin-work-dir");
}

void waitAMoment() throws InterruptedException {
Thread.yield();
Thread.sleep(2000);
}

void waitUntilNextRun(DefaultPluginJarLocationMonitor monitor) throws InterruptedException {
long previousRun = monitor.getLastRun();
int numberOfTries = 0;
while (previousRun >= monitor.getLastRun() && numberOfTries < NO_OF_TRIES_TO_CHECK_MONITOR_RUN) {
Thread.yield();
Thread.sleep(100);
numberOfTries++;
}
if (numberOfTries >= NO_OF_TRIES_TO_CHECK_MONITOR_RUN) {
throw new RuntimeException("Number of tries exceeded, but monitor thread hasn't run yet");
void copyPluginToThePluginDirectory(BundleOrPluginFileDetails plugin) throws IOException {
try (InputStream is = Objects.requireNonNull(getClass().getClassLoader().getResourceAsStream("defaultFiles/descriptor-aware-test-plugin.jar"))) {
Files.copy(is, plugin.file().toPath(), StandardCopyOption.REPLACE_EXISTING);
}
}

void copyPluginToThePluginDirectory(File pluginDir,
String destinationFilenameOfPlugin) throws IOException, URISyntaxException {
URL resource = getClass().getClassLoader().getResource("defaultFiles/descriptor-aware-test-plugin.jar");

FileUtils.copyURLToFile(resource, new File(pluginDir, destinationFilenameOfPlugin));
}

void updateFileContents(File someFile) {
try (FileOutputStream output = new FileOutputStream(someFile)) {
IOUtils.write("some rubbish", output, Charset.defaultCharset());
} catch (IOException e) {
throw new RuntimeException(e);
}
void updateFileContents(File someFile) throws IOException {
Files.writeString(someFile.toPath(), "some rubbish", StandardCharsets.UTF_8, StandardOpenOption.APPEND);
}

BundleOrPluginFileDetails pluginFileDetails(File directory, String pluginFile, boolean bundledPlugin) {
return new BundleOrPluginFileDetails(new File(directory, pluginFile), bundledPlugin, pluginWorkDir);
}

void verifyNoMoreInteractionsOtherThanPhantomUpdatesFor(BundleOrPluginFileDetails... plugins) {
// Sometimes there are phantom update events from the OS. We are not worried about these.
for (var plugin : Optional.ofNullable(plugins).orElse(new BundleOrPluginFileDetails[0])) {
verify(changeListener, atMostOnce()).pluginJarUpdated(plugin);
}
verifyNoMoreInteractions(changeListener);
}
}
Loading

0 comments on commit 457240d

Please sign in to comment.