Fix #3: Make tests run more quickly.

Add a new shell script to run the tests.  It uses a new new environment
variable `HREPL_TEST_OUTPUT` that lets the tests share the same output
directory.  Run `bazel clean` in between each test to make them essentially
hermetic (as far as this test is concerned), without redoing expensive
computations in each test such as fetching/installing the GHC build or building
the `protoc` binary.

Also set the disk cache explicitly in the test output directory to cache
build outputs across tests.

When I ran the tests with With this change, when I ran it two tests each
took a little under 5 minutes (`joined_ffi_test`, because it was the first
one, and `proto_test`, because it needed to rebuild more protobuf dependencies)
and the rest of the tests each took ~10s.  The total runtime was a little
under 10 minutes.

WORKSPACE

@@ -6,8 +6,8 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 git_repository(
     name = "rules_haskell",
     remote = "https://github.com/judah/rules_haskell",
-    commit = "fe655d5bd723380f6e4a2ee0856448afaef5d11a",
-    shallow_since = "1578765654 -0800",
+    commit = "3d6318dbdb4e5ec011f318cfa91ac562499e0604",
+    shallow_since = "1578770323 -0800",
 )
 load("@rules_haskell//haskell:repositories.bzl", "rules_haskell_dependencies")
 rules_haskell_dependencies()

hrepl/tests/BUILD.bazel

@@ -2,7 +2,10 @@
 #
 # To run the tests locally:
 #
-#   bazel test -j 4 --test_env=PATH --run_under="env HREPL_TEST_CLIENT=$PWD" hrepl/tests:hrepl_tests
+# hrepl/tests/run_tests hrepl/tests:hrepl_tests
+#
+# The first unit test may take several minutes to run as it computes the dependencies.
+# Subsequent tests in the same run should use the same build outputs and run more quickly.
 
 load("@rules_haskell//haskell:defs.bzl", "haskell_binary", "haskell_library", "ghc_plugin")
 load("@rules_haskell//haskell:protobuf.bzl", "haskell_proto_library")
@@ -26,6 +29,7 @@ haskell_library(
         "@rules_haskell//tools/runfiles",
         "@stackage//:HUnit",
         "@stackage//:base",
+        "@stackage//:directory",
         "@stackage//:filepath",
         "@stackage//:process",
         "@stackage//:strict",

hrepl/tests/ReplTestLib.hs

@@ -22,17 +22,23 @@
 -- dependencies that are not factored into publicly visible
 -- targets. Moreover, the dependencies are likely unstable as they are
 -- implementation artifacts and not designed interfaces.
+--
+-- You may also share the same output directory across runs by setting
+-- HREPL_TEST_OUTPUT.  This setting is useful in particular for caching
+-- build outputs.
 module ReplTestLib
     ( TestScript(..)
     , hreplTest
     ) where
 
 import Prelude hiding (readFile)
 
+import Control.Exception (bracket)
 import Control.Monad (unless)
-import Bazel(BazelOpts(..), bazelShutdown, defBazelOpts)
+import Bazel(BazelOpts(..), bazelClean, bazelShutdown, defBazelOpts)
 import qualified Bazel.Runfiles as Runfiles
-import System.Environment (getEnv)
+import System.Directory (getCurrentDirectory, setCurrentDirectory)
+import System.Environment (getEnv, lookupEnv)
 import System.Exit (ExitCode(..))
 import System.FilePath ((</>))
 import System.IO (hClose, hPutStrLn)
@@ -44,16 +50,25 @@ import System.Process ( CreateProcess(..), StdStream(..), proc, waitForProcess
 import Test.HUnit (assertEqual)
 import Test.HUnit.Lang (Assertion)
 
-newtype TestDirs = TestDirs
-    { baseDir :: FilePath
+data TestDirs = TestDirs
+    { clientDir :: FilePath
+    , outputDir :: FilePath
     }
 
+withTestDirs :: (TestDirs -> IO a) -> IO a
+withTestDirs act = do
+    client <- getEnv "HREPL_TEST_CLIENT"
+    let run output = act TestDirs { clientDir = client, outputDir = output }
+    maybeOutputDir <- lookupEnv "HREPL_TEST_OUTPUT"
+    case maybeOutputDir of
+        Nothing -> withSystemTempDirectory "hrepl_test_output" run
+        Just o -> run o
+
 -- | Runs the test with the given script and verifies the output
 -- printed to argv[0] matches the expected.
 hreplTest :: TestScript -> String -> Assertion
 hreplTest t expected = do
-    dirs <- TestDirs <$> getEnv "HREPL_TEST_CLIENT"
-    got <- runHrepl t dirs
+    got <- withTestDirs (runHrepl t)
     assertEqual "Unexpected result" expected got
 
 -- Parameters for runTest.
@@ -63,14 +78,14 @@ data TestScript = TestScript
     }
 
 -- | Convenient options for running bazel in isolation.
-testBazelOpts :: TestDirs -> FilePath -> BazelOpts
-testBazelOpts testDirs tmpD =
+testBazelOpts :: TestDirs -> BazelOpts
+testBazelOpts testDirs =
     defBazelOpts
     { bazelPre = bazelPre defBazelOpts ++
                  [ -- Isolates from any config settings.
                    "--bazelrc=/dev/null"
                    -- Redirect outputs to a temporary location
-                 , "--output_base=" ++ tmpD </> "output-base"
+                 , "--output_base=" ++ outputDir testDirs </> "output-base"
                  ]
     , bazelPost = bazelPost defBazelOpts ++
                 [ "--noshow_progress"
@@ -86,41 +101,53 @@ testBazelOpts testDirs tmpD =
     }
 
 runHrepl :: TestScript -> TestDirs -> IO String
-runHrepl TestScript{..} testDirs =
-    withSystemTempDirectory "hrepl_test." $ \tmpD -> do
-        rfiles <- Runfiles.create
-        path <- getEnv "PATH"
-        let hrepl = Runfiles.rlocation rfiles "hrepl/hrepl/hrepl"
-        let bazelOpts = testBazelOpts testDirs tmpD
-            args = [ "--bazel"
-                   , bazelBin bazelOpts
-                   , "--bazel-pre-args"
-                   , unwords $ bazelPre bazelOpts
-                   , "--bazel-args"
-                   , unwords $ bazelPost bazelOpts
-                   , "--show-commands"
-                   ]
-                   ++ tsUserArgs
-            cp = (proc hrepl args) { cwd = Just (baseDir testDirs)
-                                   -- TODO: don't pass PATH through.
-                                   -- It's currently needed for ghc_bindist to find the
-                                   -- `python` executable.
-                                   , env = Just [("HOME", tmpD), ("PATH", path)]
-                                   , std_in = CreatePipe
-                                   }
-        withCreateProcess cp $ \(Just stdin) _ _ ph -> do
-            -- Uses a distinct explicit result file instead of stdout or stderr.
-            -- Both of these are polluted by hrepl, bazel, and ghci.
-            let output = tmpD </> "result"
-            hPutStrLn stdin (tsStdin output)
-            hPutStrLn stdin ":quit"
-            hClose stdin
+runHrepl TestScript{..} testDirs = do
+    -- Share the same disk cache across runs in the same HREPL_OUTPUT_BASE.
+    writeFile (outputDir testDirs </> ".bazelrc")
+        $ "build --disk_cache=" ++ outputDir testDirs </> "cache"
+    rfiles <- Runfiles.create
+    path <- getEnv "PATH"
+    let hrepl = Runfiles.rlocation rfiles "hrepl/hrepl/hrepl"
+    let bazelOpts = testBazelOpts testDirs
+        args = [ "--bazel"
+                , bazelBin bazelOpts
+                , "--bazel-pre-args"
+                , unwords $ bazelPre bazelOpts
+                , "--bazel-args"
+                , unwords $ bazelPost bazelOpts
+                , "--show-commands"
+                ]
+                ++ tsUserArgs
+        cp = (proc hrepl args)
+                { cwd = Just (clientDir testDirs)
+                -- TODO: don't pass PATH through.
+                -- It's currently needed for ghc_bindist to find the
+                -- `python` executable.
+                , env = Just [("HOME", outputDir testDirs), ("PATH", path)]
+                , std_in = CreatePipe
+                }
+    -- Clean up any previous builds (such as when sharing HREPL_OUTPUT_BASE)
+    bracket getCurrentDirectory setCurrentDirectory
+        $ const $ do
+            setCurrentDirectory (clientDir testDirs)
+            bazelClean bazelOpts
+    let output = outputDir testDirs </> "result"
+    -- If the output file already exists, overwrite any previous values.
+    -- If it doesn't exist, prevent confusing "file not found" errors in case the
+    -- test fails and doesn't append anything to it.
+    writeFile output ""
+    withCreateProcess cp $ \(Just stdin) _ _ ph -> do
+        -- Uses a distinct explicit result file instead of stdout or stderr.
+        -- Both of these are polluted by hrepl, bazel, and ghci.
+        hPutStrLn stdin (tsStdin output)
+        hPutStrLn stdin ":quit"
+        hClose stdin
 
-            exitCode <- waitForProcess ph
-            unless (exitCode == ExitSuccess) $
-                error $ "Hrepl failed with: " ++ show exitCode
-            -- Shut down the async bazel process to prevent test flakiness
-            -- and zombie bazel processes.
-            bazelShutdown bazelOpts
+        exitCode <- waitForProcess ph
+        unless (exitCode == ExitSuccess) $
+            error $ "Hrepl failed with: " ++ show exitCode
+        -- Shut down the async bazel process to prevent test flakiness
+        -- and zombie bazel processes.
+        bazelShutdown bazelOpts
 
-            readFile output
+        readFile output

hrepl/tests/run_tests.sh

@@ -0,0 +1,26 @@
+#!/bin/bash
+#
+# Runs one or more tests, sharing the output cache among them.
+#
+# Usage:
+#    hrepl/tests/run_tests.sh $TARGETS
+
+
+set -ueo pipefail
+set -x
+
+# Build tests, including the hrepl binary, with the default parallelism
+bazel build "$@"
+
+OUTPUT="$(mktemp -d)"
+trap "bazel --output_base=$OUTPUT/output-base clean --expunge && rm -rf $OUTPUT" EXIT
+
+# Now run each test in sequence, using the same output directory to share the workspace
+# and cache:
+bazel test -j 1 \
+    --test_env=PATH \
+    --test_env=HREPL_TEST_CLIENT="$PWD" \
+    --test_env=HREPL_TEST_OUTPUT="$OUTPUT" \
+    --test_output=streamed \
+    -k \
+    "$@"

src/Bazel.hs

@@ -32,6 +32,7 @@ module Bazel
     getGenfilesDir,
     bazelInfo,
     bazelShutdown,
+    bazelClean,
   )
 where
 
@@ -110,12 +111,15 @@ bazelCmd_ :: BazelOpts -> String -> [String] -> IO ()
 bazelCmd_ opts cmd args = do
   allArgs <- getBazelArgs opts cmd args
   Process.withCreateProcess
-    (Process.proc (bazelBin opts) allArgs)
-      { std_err = CreatePipe,
+    (Process.proc (bazelBin opts) (allArgs ++ ["--color=yes", "--curses=yes"]))
+      { -- std_err = CreatePipe,
         cwd = bazelCwd opts
       }
+{-
     $ \Nothing Nothing (Just herr) ph -> do
       filterInfo herr
+-}
+    $ \Nothing Nothing Nothing ph -> do
       ec <- Process.waitForProcess ph
       when (ec /= ExitSuccess)
         $ error
@@ -168,7 +172,9 @@ filterInfo h = do
         | otherwise -> do
           -- An INFO: line: if it's the first consecutive one, print the
           -- contents before the "INFO:".
-          unless prevInfo $ T.hPutStr stderr l'
+          unless prevInfo $ do
+            T.hPutStr stderr l'
+            IO.hFlush stderr
           pure True
     info = "INFO:"
 
@@ -227,6 +233,9 @@ bazelInfo opts key =
   takeWhile (/= '\n') . T.unpack . T.decodeUtf8
     <$> bazelCmd opts "info" [key]
 
+bazelClean :: BazelOpts -> IO ()
+bazelClean opts = bazelCmd_ opts "clean" []
+
 bazelShutdown :: BazelOpts -> IO ()
 bazelShutdown opts = bazelCmd_ opts' "shutdown" []
   where