This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch fix-flaky-temp-dir-tests in repository https://gitbox.apache.org/repos/asf/tika.git
commit 577b2a7695659995dd08ff498a613e55eac98b36 Author: tallison <[email protected]> AuthorDate: Thu Sep 3 06:42:01 2026 -0400 Fix tests that flake when building in parallel --- .../apache/tika/pipes/grpc/TikaGrpcServerTest.java | 55 ++++++++++++++++++++-- tika-parent/pom.xml | 34 ++++++++++++- .../tika-parser-scientific-module/pom.xml | 3 +- .../tika/detect/TestContainerAwareDetector.java | 40 +++++++++++----- tika-pipes/tika-pipes-config-store-ignite/pom.xml | 2 + tika-server/tika-server-core/pom.xml | 3 +- tika-server/tika-server-standard/pom.xml | 3 +- 7 files changed, 118 insertions(+), 22 deletions(-) diff --git a/tika-grpc/src/test/java/org/apache/tika/pipes/grpc/TikaGrpcServerTest.java b/tika-grpc/src/test/java/org/apache/tika/pipes/grpc/TikaGrpcServerTest.java index 237c0c331b..7019676263 100644 --- a/tika-grpc/src/test/java/org/apache/tika/pipes/grpc/TikaGrpcServerTest.java +++ b/tika-grpc/src/test/java/org/apache/tika/pipes/grpc/TikaGrpcServerTest.java @@ -39,6 +39,8 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import java.util.stream.Stream; import com.asarkar.grpc.test.GrpcCleanupExtension; import com.asarkar.grpc.test.Resources; @@ -55,6 +57,7 @@ import io.grpc.stub.StreamObserver; import org.apache.commons.io.FileUtils; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -141,6 +144,47 @@ public class TikaGrpcServerTest { } } + private final List<TikaGrpcServerImpl> services = new ArrayList<>(); + + /** + * Every service built here is closed by {@link #closeServices()}. Only the manager that + * started a pipes-server child can delete that child's temp dir, so a service the test + * drops on the floor -- including on an assertion failure -- orphans it. + */ + private TikaGrpcServerImpl newService(Path config) throws Exception { + TikaGrpcServerImpl service = new TikaGrpcServerImpl(config.toAbsolutePath().toString()); + services.add(service); + return service; + } + + @AfterEach + void closeServices() throws Exception { + for (TikaGrpcServerImpl service : services) { + //both are idempotent, so a test that already shut down in-line is fine + service.shutdown(); + service.postShutdown(); + } + services.clear(); + assertNoOrphanedServerTempDirs(); + } + + /** + * Fails the test that leaked rather than leaving it for whoever notices the temp dir later. + * Reliable because surefire gives each module its own java.io.tmpdir and runs these + * classes one at a time. + */ + private static void assertNoOrphanedServerTempDirs() throws Exception { + Path tmp = Paths.get(System.getProperty("java.io.tmpdir")); + try (Stream<Path> paths = Files.list(tmp)) { + List<String> orphans = paths + .map(p -> p.getFileName().toString()) + .filter(n -> n.startsWith("pipes-server-")) + .sorted() + .collect(Collectors.toList()); + assertTrue(orphans.isEmpty(), "orphaned pipes-server temp dirs: " + orphans); + } + } + static final int NUM_FETCHERS_TO_CREATE = 10; @Test @@ -150,7 +194,7 @@ public class TikaGrpcServerTest { Server server = InProcessServerBuilder .forName(serverName) .directExecutor() - .addService(new TikaGrpcServerImpl(tikaConfigUnlocked.toAbsolutePath().toString())) + .addService(newService(tikaConfigUnlocked)) .build() .start(); resources.register(server, Duration.ofSeconds(10)); @@ -451,13 +495,14 @@ public class TikaGrpcServerTest { assertNotNull(reply.getMessage()); } - private static TikaGrpc.TikaBlockingStub startServer(Resources resources, Path config) + //non-static: newService tracks the service on the per-test instance so it gets closed + private TikaGrpc.TikaBlockingStub startServer(Resources resources, Path config) throws Exception { String serverName = InProcessServerBuilder.generateName(); Server server = InProcessServerBuilder .forName(serverName) .directExecutor() - .addService(new TikaGrpcServerImpl(config.toAbsolutePath().toString())) + .addService(newService(config)) .build() .start(); resources.register(server, Duration.ofSeconds(10)); @@ -481,7 +526,7 @@ public class TikaGrpcServerTest { Server server = InProcessServerBuilder .forName(serverName) .directExecutor() - .addService(new TikaGrpcServerImpl(tikaConfigUnlocked.toAbsolutePath().toString())) + .addService(newService(tikaConfigUnlocked)) .build() .start(); resources.register(server, Duration.ofSeconds(10)); @@ -544,7 +589,7 @@ public class TikaGrpcServerTest { public void testBiStream(Resources resources) throws Exception { String serverName = InProcessServerBuilder.generateName(); - TikaGrpcServerImpl tikaGrpcServerImpl = new TikaGrpcServerImpl(tikaConfigUnlocked.toAbsolutePath().toString()); + TikaGrpcServerImpl tikaGrpcServerImpl = newService(tikaConfigUnlocked); Server server = InProcessServerBuilder .forName(serverName) .directExecutor() diff --git a/tika-parent/pom.xml b/tika-parent/pom.xml index 9332897762..4b35d44456 100644 --- a/tika-parent/pom.xml +++ b/tika-parent/pom.xml @@ -310,6 +310,13 @@ <puppycrawl.version>12.3.1</puppycrawl.version> <rat.version>0.18</rat.version> <scm.version>2.2.1</scm.version> + <!-- Per-module temp dir for the surefire JVM. Keeps one module's temp files out of + every other module's view: under -T1C the modules share one java.io.tmpdir, and + a test that counts apache-tika-* there races whatever else is building + (TestContainerAwareDetector.testRemovalTempfiles flaked exactly this way). + Created by the create-test-tmpdir antrun execution below, because the JVM will + not create a missing java.io.tmpdir; it throws NoSuchFileException. --> + <tika.test.tmpdir>${project.build.directory}/test-tmp</tika.test.tmpdir> <checkstyle.configLocation>${maven.multiModuleProjectDirectory}/tika-parent/checkstyle.xml</checkstyle.configLocation> <spotless.goal>apply</spotless.goal> <spotless.header.file>${maven.multiModuleProjectDirectory}/tika-parent/license-header.txt</spotless.header.file> @@ -1506,9 +1513,34 @@ <configuration> <!-- for manual testing of i18n, try for example: -Duser.language=zh -Duser.region=CN or -Duser.language=de -Duser.country=DE --> - <argLine>-Xmx4g -Djava.awt.headless=true @{surefireArgLine}</argLine> + <!-- java.io.tmpdir MUST be quoted: argLine is split on whitespace, and the Windows + job deliberately checks out into a path containing a space. It must also be a + real -D on the command line, not <systemPropertyVariables>: Files.createTempFile + reads the startup value, so a System.setProperty after JVM start is a no-op. --> + <argLine>-Xmx4g -Djava.awt.headless=true "-Djava.io.tmpdir=${tika.test.tmpdir}" @{surefireArgLine}</argLine> </configuration> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-antrun-plugin</artifactId> + <version>${maven.antrun.version}</version> + <executions> + <execution> + <id>create-test-tmpdir</id> + <phase>process-test-resources</phase> + <goals> + <goal>run</goal> + </goals> + <configuration> + <target> + <!-- delete first so an incremental (unclean) build still starts empty --> + <delete dir="${tika.test.tmpdir}" /> + <mkdir dir="${tika.test.tmpdir}" /> + </target> + </configuration> + </execution> + </executions> + </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> diff --git a/tika-parsers/tika-parsers-extended/tika-parser-scientific-module/pom.xml b/tika-parsers/tika-parsers-extended/tika-parser-scientific-module/pom.xml index 9aeb79e0bf..46c135881d 100644 --- a/tika-parsers/tika-parsers-extended/tika-parser-scientific-module/pom.xml +++ b/tika-parsers/tika-parsers-extended/tika-parser-scientific-module/pom.xml @@ -111,7 +111,8 @@ May the gods of dependency management fix this in the future. <plugin> <artifactId>maven-surefire-plugin</artifactId> <configuration> - <argLine>-Dcom.google.protobuf.use_unsafe_pre22_gencode</argLine> + <!-- this argLine replaces tika-parent's, so it has to repeat the tmpdir isolation --> + <argLine>-Dcom.google.protobuf.use_unsafe_pre22_gencode "-Djava.io.tmpdir=${tika.test.tmpdir}"</argLine> </configuration> </plugin> <plugin> diff --git a/tika-parsers/tika-parsers-standard/tika-parsers-standard-integration-tests/src/test/java/org/apache/tika/detect/TestContainerAwareDetector.java b/tika-parsers/tika-parsers-standard/tika-parsers-standard-integration-tests/src/test/java/org/apache/tika/detect/TestContainerAwareDetector.java index 7840d8da31..2619286979 100644 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-integration-tests/src/test/java/org/apache/tika/detect/TestContainerAwareDetector.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-integration-tests/src/test/java/org/apache/tika/detect/TestContainerAwareDetector.java @@ -30,8 +30,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import java.util.Objects; import java.util.Random; +import java.util.stream.Stream; import org.apache.commons.io.IOUtils; import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; @@ -39,6 +39,7 @@ import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.apache.tika.MultiThreadedTikaTest; import org.apache.tika.Tika; @@ -49,6 +50,7 @@ import org.apache.tika.detect.zip.OpenDocumentDetector; import org.apache.tika.detect.zip.StreamingZipContainerDetector; import org.apache.tika.detect.zip.ZipContainerDetector; import org.apache.tika.exception.TikaException; +import org.apache.tika.io.TemporaryResources; import org.apache.tika.io.TikaInputStream; import org.apache.tika.metadata.HttpHeaders; import org.apache.tika.metadata.Metadata; @@ -71,6 +73,8 @@ public class TestContainerAwareDetector extends MultiThreadedTikaTest { private final StreamingZipContainerDetector streamingZipDetector = new StreamingZipContainerDetector(); + @TempDir + private Path tempDir; @AfterEach public void tearDown() throws TikaException { @@ -416,23 +420,33 @@ public class TestContainerAwareDetector extends MultiThreadedTikaTest { assertRemovalTempfiles("test-documents.zip"); } - private int countTemporaryFiles() { - //TODO: fix this. This can prevent multiple parallel builds - //from running at the same time because there can be more than one - //process writing to apache-tika-* - return Objects.requireNonNull(new File(System.getProperty("java.io.tmpdir")) - .listFiles((dir, name) -> name.startsWith("apache-tika-"))).length; + /** + * Counts spool files in {@link #tempDir}. The shared java.io.tmpdir cannot be counted + * reliably: parallel module builds (-T1C) have other JVMs writing apache-tika-* there. + */ + private int countTemporaryFiles() throws IOException { + try (Stream<Path> files = Files.list(tempDir)) { + return (int) files.count(); + } } private void assertRemovalTempfiles(String fileName) throws Exception { - int numberOfTempFiles = countTemporaryFiles(); - - try (TikaInputStream tis = TikaInputStream - .get(getResourceAsUrl("/test-documents/" + fileName))) { - detector.detect(tis, new Metadata(), new ParseContext()); + Metadata metadata = new Metadata(); + metadata.set(TikaCoreProperties.RESOURCE_NAME_KEY, fileName); + TemporaryResources tmp = new TemporaryResources(); + tmp.setTemporaryFileDirectory(tempDir); + + try (InputStream is = getClass().getResourceAsStream("/test-documents/" + fileName)) { + assertNotNull(is); + try (TikaInputStream tis = TikaInputStream.get(is, tmp, metadata)) { + detector.detect(tis, metadata, new ParseContext()); + //force the spool so this test can't pass vacuously + tis.getPath(); + assertEquals(1, countTemporaryFiles()); + } } - assertEquals(numberOfTempFiles, countTemporaryFiles()); + assertEquals(0, countTemporaryFiles()); } @Test diff --git a/tika-pipes/tika-pipes-config-store-ignite/pom.xml b/tika-pipes/tika-pipes-config-store-ignite/pom.xml index 349dee4645..b440392ea9 100644 --- a/tika-pipes/tika-pipes-config-store-ignite/pom.xml +++ b/tika-pipes/tika-pipes-config-store-ignite/pom.xml @@ -144,7 +144,9 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> <configuration> + <!-- this argLine replaces tika-parent's, so it has to repeat the tmpdir isolation --> <argLine> + "-Djava.io.tmpdir=${tika.test.tmpdir}" --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED diff --git a/tika-server/tika-server-core/pom.xml b/tika-server/tika-server-core/pom.xml index fd60838c42..6410e9e0d3 100644 --- a/tika-server/tika-server-core/pom.xml +++ b/tika-server/tika-server-core/pom.xml @@ -152,7 +152,8 @@ <artifactId>maven-surefire-plugin</artifactId> <configuration> <redirectTestOutputToFile>true</redirectTestOutputToFile> - <argLine>-da -XX:+HeapDumpOnOutOfMemoryError -Xmx512m</argLine> + <!-- this argLine replaces tika-parent's, so it has to repeat the tmpdir isolation --> + <argLine>-da -XX:+HeapDumpOnOutOfMemoryError -Xmx512m "-Djava.io.tmpdir=${tika.test.tmpdir}"</argLine> <systemPropertyVariables> <java.util.logging.config.file> ${basedir}/src/main/resources/commons-logging.properties diff --git a/tika-server/tika-server-standard/pom.xml b/tika-server/tika-server-standard/pom.xml index 32fea14da8..a0bd28e946 100644 --- a/tika-server/tika-server-standard/pom.xml +++ b/tika-server/tika-server-standard/pom.xml @@ -122,7 +122,8 @@ <artifactId>maven-surefire-plugin</artifactId> <configuration> <redirectTestOutputToFile>true</redirectTestOutputToFile> - <argLine>-da -XX:+HeapDumpOnOutOfMemoryError -Xmx512m</argLine> + <!-- this argLine replaces tika-parent's, so it has to repeat the tmpdir isolation --> + <argLine>-da -XX:+HeapDumpOnOutOfMemoryError -Xmx512m "-Djava.io.tmpdir=${tika.test.tmpdir}"</argLine> <systemPropertyVariables> <java.util.logging.config.file> ${basedir}/src/main/resources/log4j2.xml
