fix MercurialRepositoryTest.testBuildTagListOneMore failure with Mercurial 7.1.1 on macOS (#4850)

this avoids interactive mode and errors due to no user being set

fixes #4849

Author: vladak

opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java

@@ -33,6 +33,7 @@
 import java.io.Reader;
 import java.lang.Thread.UncaughtExceptionHandler;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -67,6 +68,7 @@ public class Executor {
     private byte[] stdout;
     private byte[] stderr;
     private int timeout; // in milliseconds, 0 means no timeout
+    private final Map<String, String> environ;
 
     /**
      * Create a new instance of the Executor.
@@ -88,47 +90,65 @@ public Executor(List<String> cmdList) {
      * Create a new instance of the Executor with default command timeout value.
      * The timeout value will be based on the running context (indexer or web application).
      * @param cmdList A list containing the command to execute
-     * @param workingDirectory The directory the process should have as the
-     *                         working directory
+     * @param workingDirectory The directory the process should have as the working directory
      */
-    public Executor(List<String> cmdList, File workingDirectory) {
-        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
-        int timeoutSec = env.isIndexer() ? env.getIndexerCommandTimeout() : env.getInteractiveCommandTimeout();
-
-        this.cmdList = cmdList;
-        this.workingDirectory = workingDirectory;
-        this.timeout = timeoutSec * 1000;
+    public Executor(List<String> cmdList, @Nullable File workingDirectory) {
+        this(cmdList, workingDirectory, new HashMap<>());
     }
 
     /**
      * Create a new instance of the Executor with specific timeout value.
      * @param cmdList A list containing the command to execute
-     * @param workingDirectory The directory the process should have as the
-     *                         working directory
+     * @param workingDirectory The directory the process should have as the working directory
      * @param timeout If the command runs longer than the timeout (seconds),
      *                it will be terminated. If the value is 0, no timer
      *                will be set up.
      */
-    public Executor(List<String> cmdList, File workingDirectory, int timeout) {
+    public Executor(List<String> cmdList, @Nullable File workingDirectory, int timeout) {
+        this(cmdList, workingDirectory, new HashMap<>());
+
+        this.timeout = timeout * 1000;
+    }
+
+    /**
+     * Create a new instance of the Executor with default command timeout value and environment.
+     * The timeout value will be based on the running context (indexer or web application).
+     * @param cmdList A list containing the command to execute
+     * @param workingDirectory The directory the process should have as the working directory
+     * @param environ map of environment variables to be added/overridden
+     */
+    public Executor(List<String> cmdList, @Nullable File workingDirectory, Map<String, String> environ) {
         this.cmdList = cmdList;
         this.workingDirectory = workingDirectory;
-        this.timeout = timeout * 1000;
+        this.environ = environ;
+
+        setDefaultTimeout();
     }
 
     /**
      * Create a new instance of the Executor with or without timeout.
      * @param cmdList A list containing the command to execute
-     * @param workingDirectory The directory the process should have as the
-     *                         working directory
+     * @param workingDirectory The directory the process should have as the working directory
      * @param useTimeout terminate the process after default timeout or not
      */
-    public Executor(List<String> cmdList, File workingDirectory, boolean useTimeout) {
+    public Executor(List<String> cmdList, @Nullable File workingDirectory, boolean useTimeout) {
         this(cmdList, workingDirectory);
         if (!useTimeout) {
             this.timeout = 0;
         }
     }
 
+    private void setDefaultTimeout() {
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+        int timeoutSec = env.isIndexer() ? env.getIndexerCommandTimeout() : env.getInteractiveCommandTimeout();
+
+        this.timeout = timeoutSec * 1000;
+    }
+
+    int getTimeout() {
+        return timeout / 1000;
+    }
+
     /**
      * Execute the command and collect the output. All exceptions will be
      * logged.
@@ -180,6 +200,8 @@ public int exec(final boolean reportExceptions, StreamHandler handler) {
                 .map(File::toString)
                 .orElseGet(() -> System.getProperty("user.dir"));
 
+        processBuilder.environment().putAll(environ);
+
         String envStr = "";
         if (LOGGER.isLoggable(Level.FINER)) {
             Map<String, String> envMap = processBuilder.environment();
@@ -222,7 +244,7 @@ public int exec(final boolean reportExceptions, StreamHandler handler) {
                     @Override public void run() {
                         LOGGER.log(Level.WARNING,
                             String.format("Terminating process of command [%s] in directory '%s' " +
-                            "due to timeout %d seconds", cmd_str, dir_str, timeout / 1000));
+                            "due to timeout %d seconds", cmd_str, dir_str, getTimeout()));
                         proc.destroy();
                     }
                 }, timeout);

opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java

@@ -50,7 +50,9 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
@@ -187,15 +189,18 @@ void testGetHistoryPartial() throws Exception {
      * @param args {@code hg} command arguments
      */
     public static void runHgCommand(File reposRoot, String... args) {
-        List<String> cmdargs = new ArrayList<>();
+        List<String> commandWithArgs = new ArrayList<>();
         MercurialRepository repo = new MercurialRepository();
 
-        cmdargs.add(repo.getRepoCommand());
-        cmdargs.addAll(Arrays.asList(args));
+        commandWithArgs.add(repo.getRepoCommand());
+        commandWithArgs.addAll(Arrays.asList(args));
 
-        Executor exec = new Executor(cmdargs, reposRoot);
+        final Map<String, String> env = new HashMap<>();
+        // Set the user to record commits in order to prevent hg commands entering interactive mode.
+        env.put("HGUSER", "Snufkin <[email protected]>");
+        Executor exec = new Executor(commandWithArgs, reposRoot, env);
         int exitCode = exec.exec();
-        assertEquals(0, exitCode, "hg command '" + cmdargs + "' failed."
+        assertEquals(0, exitCode, "command '" + commandWithArgs + "' failed."
                 + "\nexit code: " + exitCode
                 + "\nstdout:\n" + exec.getOutputString()
                 + "\nstderr:\n" + exec.getErrorString());
@@ -299,10 +304,7 @@ void testGetHistoryGetRenamed() throws Exception {
         String exp_str = "This is totally plaintext file.\n";
         byte[] buffer = new byte[1024];
 
-        /*
-         * In our test repository the file was renamed twice since
-         * revision 3.
-         */
+        // In our test repository the file was renamed twice since revision 3.
         InputStream input = mr.getHistoryGet(repositoryRoot.getCanonicalPath(),
                 "novel.txt", "3");
         assertNotNull(input);
@@ -350,8 +352,7 @@ void testGetHistorySinceTillNullRev() throws Exception {
         assertNotNull(history);
         assertNotNull(history.getHistoryEntries());
         assertEquals(6, history.getHistoryEntries().size());
-        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).
-                collect(Collectors.toList());
+        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList();
         assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 4, REVISIONS.length)), revisions);
     }
 
@@ -362,8 +363,7 @@ void testGetHistorySinceTillRevNull() throws Exception {
         assertNotNull(history);
         assertNotNull(history.getHistoryEntries());
         assertEquals(3, history.getHistoryEntries().size());
-        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).
-                collect(Collectors.toList());
+        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList();
         assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 0, 3)), revisions);
     }
 
@@ -374,8 +374,7 @@ void testGetHistorySinceTillRevRev() throws Exception {
         assertNotNull(history);
         assertNotNull(history.getHistoryEntries());
         assertEquals(5, history.getHistoryEntries().size());
-        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).
-                collect(Collectors.toList());
+        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList();
         assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 2, 7)), revisions);
     }
 
@@ -389,8 +388,7 @@ void testGetHistoryRenamedFileTillRev() throws Exception {
         assertNotNull(history);
         assertNotNull(history.getHistoryEntries());
         assertEquals(5, history.getHistoryEntries().size());
-        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).
-                collect(Collectors.toList());
+        List<String> revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList();
         assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 2, 7)), revisions);
     }
 
@@ -443,8 +441,10 @@ void testAnnotationNegative() throws Exception {
     }
 
     private static Stream<Triple<String, List<String>, List<String>>> provideParametersForPositiveAnnotationTest() {
-        return Stream.of(Triple.of("8:6a8c423f5624", List.of("7", "8"), List.of("8:6a8c423f5624", "7:db1394c05268")),
-                Triple.of("7:db1394c05268", List.of("7"), List.of("7:db1394c05268")));
+        return Stream.of(
+                Triple.of("8:6a8c423f5624", List.of("7", "8"), List.of("8:6a8c423f5624", "7:db1394c05268")),
+                Triple.of("7:db1394c05268", List.of("7"), List.of("7:db1394c05268"))
+        );
     }
 
     @ParameterizedTest
@@ -579,7 +579,7 @@ void testBuildTagListOneMore() throws Exception {
         assertNotNull(tags);
         assertEquals(3, tags.size());
         expectedTags = List.of("start_of_novel", newTagName, branchTagName);
-        assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).collect(Collectors.toList()));
+        assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).toList());
 
         // cleanup
         IOUtils.removeRecursive(repositoryRootPath);

opengrok-indexer/src/test/java/org/opengrok/indexer/util/ExecutorTest.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2025, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2019, Chris Fraire <[email protected]>.
  */
 package org.opengrok.indexer.util;
@@ -30,9 +30,12 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.Reader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -45,6 +48,13 @@
  */
 class ExecutorTest {
 
+    @Test
+    void testConstructorWithTimeout() {
+        int timeout = 42;
+        Executor executor = new Executor(List.of("foo"), null, timeout);
+        assertEquals(timeout, executor.getTimeout());
+    }
+
     @Test
     void testString() {
         List<String> cmdList = new ArrayList<>();
@@ -64,10 +74,14 @@ void testReader() throws IOException {
         cmdList.add("testing org.opengrok.indexer.util.Executor");
         Executor instance = new Executor(cmdList);
         assertEquals(0, instance.exec());
-        BufferedReader in = new BufferedReader(instance.getOutputReader());
+        Reader outputReader = instance.getOutputReader();
+        assertNotNull(outputReader);
+        BufferedReader in = new BufferedReader(outputReader);
         assertEquals("testing org.opengrok.indexer.util.Executor", in.readLine());
         in.close();
-        in = new BufferedReader(instance.getErrorReader());
+        Reader errorReader = instance.getErrorReader();
+        assertNotNull(errorReader);
+        in = new BufferedReader(errorReader);
         assertNull(in.readLine());
         in.close();
     }
@@ -81,14 +95,44 @@ void testStream() throws IOException {
         assertEquals(0, instance.exec());
         assertNotNull(instance.getOutputStream());
         assertNotNull(instance.getErrorStream());
-        BufferedReader in = new BufferedReader(instance.getOutputReader());
+        Reader outputReader = instance.getOutputReader();
+        assertNotNull(outputReader);
+        BufferedReader in = new BufferedReader(outputReader);
         assertEquals("testing org.opengrok.indexer.util.Executor", in.readLine());
         in.close();
-        in = new BufferedReader(instance.getErrorReader());
+        Reader errorReader = instance.getErrorReader();
+        assertNotNull(errorReader);
+        in = new BufferedReader(errorReader);
         assertNull(in.readLine());
         in.close();
     }
 
+    /**
+     * Test setting environment variable. Assumes the {@code env} program exists
+     * and reports list of environment variables along with their values.
+     */
+    @Test
+    void testEnv() throws IOException {
+        List<String> cmdList = List.of("env");
+        final Map<String, String> env = new HashMap<>();
+        env.put("foo", "bar");
+        Executor instance = new Executor(cmdList, null, env);
+        assertEquals(0, instance.exec());
+        Reader outputReader = instance.getOutputReader();
+        assertNotNull(outputReader);
+        BufferedReader in = new BufferedReader(outputReader);
+        String line;
+        boolean found = false;
+        while ((line = in.readLine()) != null) {
+            if (line.equals("foo=bar")) {
+                found = true;
+                break;
+            }
+        }
+        assertTrue(found);
+        in.close();
+    }
+
     /**
      * {@link Executor.StreamHandler} implementation that always fails with {@link IOException}.
      */