Fix Local Smoke Tests by Allowing ScriptSessionCacheInit's mkdir to R…

…ace in Parallel Tests (deephaven#5465) Co-authored-by: Ryan Caudy <ryan@deephaven.io>
rcaudy · May 14, 2024 · d621b2f · d621b2f
1 parent 7a73398
commit d621b2f
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 19 deletions.
diff --git a/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java b/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java
@@ -26,6 +26,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
 import java.util.function.Function;
@@ -41,26 +42,27 @@ public abstract class AbstractScriptSession<S extends AbstractScriptSession.Snap
 
     private static final Path CLASS_CACHE_LOCATION = CacheDir.get().resolve("script-session-classes");
 
-    protected static File newClassCacheLocation() {
+    protected static Path newClassCacheLocation() {
         // TODO(deephaven-core#1713): Introduce instance-id concept
         final UUID scriptCacheId = UuidCreator.getRandomBased();
-        final File directory = CLASS_CACHE_LOCATION.resolve(UuidCreator.toString(scriptCacheId)).toFile();
+        final Path directory = CLASS_CACHE_LOCATION.resolve(UuidCreator.toString(scriptCacheId));
         createOrClearDirectory(directory);
         return directory;
     }
 
     public static void createScriptCache() {
-        final File classCacheDirectory = CLASS_CACHE_LOCATION.toFile();
-        createOrClearDirectory(classCacheDirectory);
+        createOrClearDirectory(CLASS_CACHE_LOCATION);
     }
 
-    private static void createOrClearDirectory(final File directory) {
-        if (directory.exists()) {
-            FileUtils.deleteRecursively(directory);
+    private static void createOrClearDirectory(final Path directory) {
+        if (Files.exists(directory)) {
+            FileUtils.deleteRecursively(directory.toFile());
         }
-        if (!directory.mkdirs()) {
+        try {
+            Files.createDirectories(directory);
+        } catch (IOException e) {
             throw new UncheckedDeephavenException(
-                    "Failed to create class cache directory " + directory.getAbsolutePath());
+                    "Failed to create class cache directory " + directory.toAbsolutePath(), e);
         }
     }
 
@@ -78,7 +80,7 @@ protected AbstractScriptSession(
             final OperationInitializer operationInitializer,
             final ObjectTypeLookup objectTypeLookup,
             @Nullable final Listener changeListener) {
-        this(updateGraph, operationInitializer, objectTypeLookup, changeListener, newClassCacheLocation(),
+        this(updateGraph, operationInitializer, objectTypeLookup, changeListener, newClassCacheLocation().toFile(),
                 Thread.currentThread().getContextClassLoader());
     }
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
@@ -187,7 +187,7 @@ public static GroovyDeephavenSession of(
             addDefaultImports(loadedGroovyScriptImports);
         }
 
-        final File classCacheDirectory = AbstractScriptSession.newClassCacheLocation();
+        final File classCacheDirectory = AbstractScriptSession.newClassCacheLocation().toFile();
 
         // Specify a classloader to read from the classpath, with script imports
         CompilerConfiguration scriptConfig = new CompilerConfiguration();

diff --git a/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java b/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java
@@ -7,7 +7,6 @@
 import dagger.Component;
 import io.deephaven.client.ClientDefaultsModule;
 import io.deephaven.engine.context.ExecutionContext;
-import io.deephaven.engine.context.TestExecutionContext;
 import io.deephaven.engine.liveness.LivenessScope;
 import io.deephaven.engine.liveness.LivenessScopeStack;
 import io.deephaven.engine.updategraph.impl.PeriodicUpdateGraph;
@@ -41,7 +40,6 @@
 import java.io.PrintStream;
 import java.time.Duration;
 import java.util.Optional;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Manages a single instance of {@link DeephavenApiServer}.
@@ -145,7 +143,10 @@ public void setUp() throws Exception {
 
     @After
     public void tearDown() throws Exception {
-        scopeCloseable.close();
+        if (scopeCloseable != null) {
+            scopeCloseable.close();
+            scopeCloseable = null;
+        }
 
         try {
             server.teardownForUnitTests();

diff --git a/server/src/test/java/io/deephaven/server/table/ops/GrpcTableOperationTestBase.java b/server/src/test/java/io/deephaven/server/table/ops/GrpcTableOperationTestBase.java
@@ -38,12 +38,16 @@ public void setUp() throws Exception {
 
     @Override
     public void tearDown() throws Exception {
-        for (ExportObject<?> export : exports) {
-            export.cancel();
+        if (exports != null) {
+            for (ExportObject<?> export : exports) {
+                export.cancel();
+            }
+            exports = null;
+        }
+        if (executionContext != null) {
+            executionContext.close();
+            executionContext = null;
         }
-        exports = null;
-        executionContext.close();
-        executionContext = null;
         super.tearDown();
     }