diff --git a/.stan.toml b/.stan.toml index f5108a97ce..066f2ef802 100644 --- a/.stan.toml +++ b/.stan.toml @@ -151,7 +151,7 @@ # Anti-pattern: Data.ByteString.Char8.pack [[ignore]] - id = "OBS-STAN-0203-tuE+RG-250:24" + id = "OBS-STAN-0203-tuE+RG-252:24" # ✦ Description: Usage of 'pack' function that doesn't handle Unicode characters # ✦ Category: #AntiPattern # ✦ File: src\Stack\Build\ExecutePackage.hs @@ -269,7 +269,7 @@ # Anti-pattern: unsafe functions [[ignore]] - id = "OBS-STAN-0212-FNS1cF-80:14" + id = "OBS-STAN-0212-FNS1cF-81:14" # ✦ Description: Usage of unsafe functions breaks referential transparency # ✦ Category: #Unsafe #AntiPattern # ✦ File: src\Stack\BuildOpts.hs @@ -280,7 +280,7 @@ # Anti-pattern: unsafe functions [[ignore]] - id = "OBS-STAN-0212-FNS1cF-91:15" + id = "OBS-STAN-0212-FNS1cF-92:15" # ✦ Description: Usage of unsafe functions breaks referential transparency # ✦ Category: #Unsafe #AntiPattern # ✦ File: src/Stack/BuildOpts.hs diff --git a/ChangeLog.md b/ChangeLog.md index 034489bd67..2784db0590 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -23,6 +23,10 @@ Other enhancements: * Add option `--reach ` to Stack's `dot` and `ls dependencies` commands, to prune packages that cannot reach any of the specified packages in the dependency graph. +* Add option `--test-suite-timeout-grace ARG` (and corresponding non-project + configuration key `test-suite-timeout-grace`) to pair with + `--test-suite-timeout ARG` and perform staged timeout termination for test + suites. Bug fixes: diff --git a/doc/commands/bench_command.md b/doc/commands/bench_command.md index 52ff529810..0498b7e70d 100644 --- a/doc/commands/bench_command.md +++ b/doc/commands/bench_command.md @@ -17,6 +17,7 @@ stack bench [TARGET] [--dry-run] [--pedantic] [--fast] [--ghc-options OPTIONS] [--[no-]keep-going] [--[no-]keep-tmp-files] [--[no-]force-dirty] [--[no-]test] [--[no-]rerun-tests] [--ta|--test-arguments TEST_ARGS] [--coverage] [--[no-]run-tests] [--test-suite-timeout ARG] + [--test-suite-timeout-grace ARG] [--[no-]tests-allow-stdin] [--[no-]bench] [--ba|--benchmark-arguments BENCH_ARGS] [--[no-]run-benchmarks] [--[no-]reconfigure] [--cabal-verbosity VERBOSITY | diff --git a/doc/commands/build_command.md b/doc/commands/build_command.md index 8b483c1de5..70b0730952 100644 --- a/doc/commands/build_command.md +++ b/doc/commands/build_command.md @@ -18,6 +18,7 @@ stack build [TARGET] [--dry-run] [--pedantic] [--fast] [--ghc-options OPTIONS] [--[no-]keep-going] [--[no-]keep-tmp-files] [--[no-]force-dirty] [--[no-]test] [--[no-]rerun-tests] [--ta|--test-arguments TEST_ARGS] [--coverage] [--[no-]run-tests] [--test-suite-timeout ARG] + [--test-suite-timeout-grace ARG] [--[no-]tests-allow-stdin] [--[no-]bench] [--ba|--benchmark-arguments BENCH_ARGS] [--[no-]run-benchmarks] [--[no-]reconfigure] [--cabal-verbosity VERBOSITY | @@ -826,6 +827,16 @@ suite in a timeout so that the test suite fails if no result is available within the specified number of seconds. The option is ignored if the number of seconds is not positive. +### `--test-suite-timeout-grace` option + +Default: None + +`stack build --test --test-suite-timeout= --test-suite-timeout-grace=` +uses staged timeout termination for each running test suite: after +`--test-suite-timeout` is reached, Stack waits the specified grace period before +force termination. The option is ignored if the number of seconds is not +positive. + ## Flags affecting GHC's behaviour ### `--[no-]executable-profiling` flag diff --git a/doc/commands/haddock_command.md b/doc/commands/haddock_command.md index e1c5bff14a..362eed0af1 100644 --- a/doc/commands/haddock_command.md +++ b/doc/commands/haddock_command.md @@ -17,7 +17,8 @@ stack haddock [TARGET] [--dry-run] [--pedantic] [--fast] [--ghc-options OPTIONS] [--[no-]keep-going] [--[no-]keep-tmp-files] [--[no-]force-dirty] [--[no-]test] [--[no-]rerun-tests] [--ta|--test-arguments TEST_ARGS] [--coverage] [--[no-]run-tests] - [--test-suite-timeout ARG] [--[no-]tests-allow-stdin] + [--test-suite-timeout ARG] [--test-suite-timeout-grace ARG] + [--[no-]tests-allow-stdin] [--[no-]bench] [--ba|--benchmark-arguments BENCH_ARGS] [--[no-]run-benchmarks] [--[no-]reconfigure] [--cabal-verbosity VERBOSITY | --[no-]cabal-verbose] diff --git a/doc/commands/install_command.md b/doc/commands/install_command.md index 653981a2d4..f579bcc4a3 100644 --- a/doc/commands/install_command.md +++ b/doc/commands/install_command.md @@ -17,7 +17,8 @@ stack install [TARGET] [--dry-run] [--pedantic] [--fast] [--ghc-options OPTIONS] [--[no-]keep-going] [--[no-]keep-tmp-files] [--[no-]force-dirty] [--[no-]test] [--[no-]rerun-tests] [--ta|--test-arguments TEST_ARGS] [--coverage] [--[no-]run-tests] - [--test-suite-timeout ARG] [--[no-]tests-allow-stdin] + [--test-suite-timeout ARG] [--test-suite-timeout-grace ARG] + [--[no-]tests-allow-stdin] [--[no-]bench] [--ba|--benchmark-arguments BENCH_ARGS] [--[no-]run-benchmarks] [--[no-]reconfigure] [--cabal-verbosity VERBOSITY | --[no-]cabal-verbose] diff --git a/doc/commands/test_command.md b/doc/commands/test_command.md index cb0f2cb9c9..bd740116f7 100644 --- a/doc/commands/test_command.md +++ b/doc/commands/test_command.md @@ -17,6 +17,7 @@ stack test [TARGET] [--dry-run] [--pedantic] [--fast] [--ghc-options OPTIONS] [--[no-]keep-going] [--[no-]keep-tmp-files] [--[no-]force-dirty] [--[no-]test] [--[no-]rerun-tests] [--ta|--test-arguments TEST_ARGS] [--coverage] [--[no-]run-tests] [--test-suite-timeout ARG] + [--test-suite-timeout-grace ARG] [--[no-]tests-allow-stdin] [--[no-]bench] [--ba|--benchmark-arguments BENCH_ARGS] [--[no-]run-benchmarks] [--[no-]reconfigure] [--cabal-verbosity VERBOSITY | diff --git a/doc/configure/yaml/non-project.md b/doc/configure/yaml/non-project.md index 5c2e14b2c5..594b32cfc8 100644 --- a/doc/configure/yaml/non-project.md +++ b/doc/configure/yaml/non-project.md @@ -216,6 +216,9 @@ build: no-run-tests: false # The option is ignored if the specified number of seconds is not positive: test-suite-timeout: 0 + # Grace period in seconds after test-suite-timeout before force kill. + # Ignored if not positive, or if test-suite-timeout is not positive. + test-suite-timeout-grace: 0 bench: false benchmark-opts: diff --git a/src/Stack/Build/ExecutePackage.hs b/src/Stack/Build/ExecutePackage.hs index b03d1d2b02..04804ebd86 100644 --- a/src/Stack/Build/ExecutePackage.hs +++ b/src/Stack/Build/ExecutePackage.hs @@ -68,6 +68,8 @@ import Stack.Build.ExecuteEnv ( ExcludeTHLoading (..), ExecuteEnv (..), KeepOutputOpen (..) , OutputType (..), withSingleContext ) +import Stack.Build.TestSuiteTimeout + ( forceKill, prepareForEscalation, terminateGracefully ) import Stack.Build.Source ( addUnlistedToBuildCache ) import Stack.Config.ConfigureScript ( ensureConfigureScript ) import Stack.ConfigureOpts @@ -1156,13 +1158,47 @@ singleTest topts testsToRun ac ee task installedMap = do ) createSource OTLogFile _ h -> Nothing <$ useHandleOpen h - optionalTimeout action + runOutput p = + case (getStdout p, getStderr p) of + (Nothing, Nothing) -> pure () + (Just x, Just y) -> concurrently_ x y + (x, y) -> assert False $ + concurrently_ + (fromMaybe (pure ()) x) + (fromMaybe (pure ()) y) + timeoutWithGrace p maxSecs graceSecs = do + mExit <- timeout (maxSecs * 1000000) (waitExitCode p) + case mExit of + Just ec -> pure (Just ec) + Nothing -> do + terminateGracefully p + mGraceExit <- timeout (graceSecs * 1000000) + (waitExitCode p) + case mGraceExit of + Just _ -> pure Nothing + Nothing -> do + forceKill p + void $ waitExitCode p + pure Nothing + runWithTimeout pc + | Just maxSecs <- topts.maximumTimeSeconds, maxSecs > 0 + , Just graceSecs <- topts.timeoutGraceSeconds + , graceSecs > 0 = + withProcessWait (prepareForEscalation pc) $ \p -> do + (_, mec') <- concurrently + (runOutput p) + (timeoutWithGrace p maxSecs graceSecs) + pure mec' | Just maxSecs <- topts.maximumTimeSeconds, maxSecs > 0 = - timeout (maxSecs * 1000000) action - | otherwise = Just <$> action + timeout (maxSecs * 1000000) $ + withProcessWait pc $ \p -> do + runOutput p + waitExitCode p + | otherwise = + Just <$> withProcessWait pc (\p -> runOutput p *> waitExitCode p) mec <- withWorkingDir (toFilePath pkgDir) $ - optionalTimeout $ proc (toFilePath exePath) args $ \pc0 -> do + proc (toFilePath exePath) args $ \pc0 -> do changeStdin <- if isTestTypeLib then do @@ -1185,15 +1221,7 @@ singleTest topts testsToRun ac ee task installedMap = do $ setStdout output $ setStderr output pc0 - withProcessWait pc $ \p -> do - case (getStdout p, getStderr p) of - (Nothing, Nothing) -> pure () - (Just x, Just y) -> concurrently_ x y - (x, y) -> assert False $ - concurrently_ - (fromMaybe (pure ()) x) - (fromMaybe (pure ()) y) - waitExitCode p + runWithTimeout pc -- Add a trailing newline, incase the test -- output didn't finish with a newline. case outputType of diff --git a/src/Stack/BuildOpts.hs b/src/Stack/BuildOpts.hs index 649c26250e..b843a5b4a8 100644 --- a/src/Stack/BuildOpts.hs +++ b/src/Stack/BuildOpts.hs @@ -74,6 +74,7 @@ defaultTestOpts = TestOpts , coverage = defaultFirstFalse toMonoid.coverage , runTests = defaultFirstTrue toMonoid.runTests , maximumTimeSeconds = Nothing + , timeoutGraceSeconds = Nothing , allowStdin = defaultFirstTrue toMonoid.allowStdin } where diff --git a/src/Stack/Config/Build.hs b/src/Stack/Config/Build.hs index afd020a9f8..3731527744 100644 --- a/src/Stack/Config/Build.hs +++ b/src/Stack/Config/Build.hs @@ -135,6 +135,10 @@ testOptsFromMonoid toMonoid madditional = defaultTestOpts fromFirst defaultTestOpts.maximumTimeSeconds toMonoid.maximumTimeSeconds + , TestOpts.timeoutGraceSeconds = + fromFirst + defaultTestOpts.timeoutGraceSeconds + toMonoid.timeoutGraceSeconds , TestOpts.allowStdin = fromFirstTrue toMonoid.allowStdin } diff --git a/src/Stack/Options/TestParser.hs b/src/Stack/Options/TestParser.hs index 1a708e9ca6..d6171ddc50 100644 --- a/src/Stack/Options/TestParser.hs +++ b/src/Stack/Options/TestParser.hs @@ -47,6 +47,11 @@ testOptsParser hide0 = TestOptsMonoid <> help "Maximum test suite run time in seconds." <> hide )) + <*> optionalFirst (option (fmap Just auto) + ( long "test-suite-timeout-grace" + <> help "Grace period in seconds after timeout before force termination." + <> hide + )) <*> firstBoolFlagsTrue "tests-allow-stdin" "allow standard input in test suites." diff --git a/src/Stack/Types/BuildOpts.hs b/src/Stack/Types/BuildOpts.hs index bdff5a886a..cf7c335db8 100644 --- a/src/Stack/Types/BuildOpts.hs +++ b/src/Stack/Types/BuildOpts.hs @@ -115,6 +115,8 @@ data TestOpts = TestOpts , coverage :: !Bool -- ^ Generate a code coverage report , runTests :: !Bool -- ^ Enable running of tests , maximumTimeSeconds :: !(Maybe Int) -- ^ test suite timeout in seconds + , timeoutGraceSeconds :: !(Maybe Int) + -- ^ additional grace period after timeout before force-killing , allowStdin :: !Bool -- ^ Whether to allow standard input } deriving (Eq, Show) diff --git a/src/Stack/Types/BuildOptsMonoid.hs b/src/Stack/Types/BuildOptsMonoid.hs index a6f378f60c..26847527bb 100644 --- a/src/Stack/Types/BuildOptsMonoid.hs +++ b/src/Stack/Types/BuildOptsMonoid.hs @@ -274,6 +274,7 @@ data TestOptsMonoid = TestOptsMonoid , coverage :: !FirstFalse , runTests :: !FirstTrue , maximumTimeSeconds :: !(First (Maybe Int)) + , timeoutGraceSeconds :: !(First (Maybe Int)) , allowStdin :: !FirstTrue } deriving (Show, Generic) @@ -285,6 +286,7 @@ instance FromJSON (WithJSONWarnings TestOptsMonoid) where coverage <- FirstFalse <$> o ..:? coverageArgName runTests <- FirstTrue . (not <$>) <$> o ..:? noRunTestsArgName maximumTimeSeconds <- First <$> o ..:? maximumTimeSecondsArgName + timeoutGraceSeconds <- First <$> o ..:? timeoutGraceSecondsArgName allowStdin <- FirstTrue <$> o ..:? testsAllowStdinName pure TestOptsMonoid { rerunTests @@ -292,6 +294,7 @@ instance FromJSON (WithJSONWarnings TestOptsMonoid) where , coverage , runTests , maximumTimeSeconds + , timeoutGraceSeconds , allowStdin } @@ -310,6 +313,9 @@ noRunTestsArgName = "no-run-tests" maximumTimeSecondsArgName :: Text maximumTimeSecondsArgName = "test-suite-timeout" +timeoutGraceSecondsArgName :: Text +timeoutGraceSecondsArgName = "test-suite-timeout-grace" + testsAllowStdinName :: Text testsAllowStdinName = "tests-allow-stdin" diff --git a/src/unix/Stack/Build/TestSuiteTimeout.hs b/src/unix/Stack/Build/TestSuiteTimeout.hs new file mode 100644 index 0000000000..e496a661d9 --- /dev/null +++ b/src/unix/Stack/Build/TestSuiteTimeout.hs @@ -0,0 +1,40 @@ +{-# LANGUAGE NoImplicitPrelude #-} + +{-| +Module : Stack.Build.TestSuiteTimeout +Description : OS-specific test suite timeout termination helpers. +License : BSD-3-Clause +-} + +module Stack.Build.TestSuiteTimeout + ( prepareForEscalation + , terminateGracefully + , forceKill + ) where + +import RIO.Process ( ProcessConfig, setNewSession ) +import qualified RIO.Process as RP ( Process, unsafeProcessHandle ) +import Stack.Prelude +import System.Posix.Signals + ( sigKILL, sigTERM, signalProcess, signalProcessGroup ) +import qualified System.Process as Process + +prepareForEscalation :: ProcessConfig stdin stdout stderr -> ProcessConfig stdin stdout stderr +prepareForEscalation = setNewSession True + +terminateGracefully :: RP.Process stdin stdout stderr -> RIO env () +terminateGracefully p = do + let processHandle = RP.unsafeProcessHandle p + mpid <- liftIO $ Process.getPid processHandle + forM_ mpid $ \pid -> do + -- In a new session, the initial pid is also the process group id. + void $ tryAny $ liftIO $ signalProcessGroup sigTERM pid + void $ tryAny $ liftIO $ signalProcess sigTERM pid + +forceKill :: RP.Process stdin stdout stderr -> RIO env () +forceKill p = do + let processHandle = RP.unsafeProcessHandle p + mpid <- liftIO $ Process.getPid processHandle + forM_ mpid $ \pid -> do + void $ tryAny $ liftIO $ signalProcessGroup sigKILL pid + void $ tryAny $ liftIO $ signalProcess sigKILL pid diff --git a/src/windows/Stack/Build/TestSuiteTimeout.hs b/src/windows/Stack/Build/TestSuiteTimeout.hs new file mode 100644 index 0000000000..2603b01ce1 --- /dev/null +++ b/src/windows/Stack/Build/TestSuiteTimeout.hs @@ -0,0 +1,29 @@ +{-# LANGUAGE NoImplicitPrelude #-} + +{-| +Module : Stack.Build.TestSuiteTimeout +Description : OS-specific test suite timeout termination helpers. +License : BSD-3-Clause +-} + +module Stack.Build.TestSuiteTimeout + ( prepareForEscalation + , terminateGracefully + , forceKill + ) where + +import RIO.Process ( ProcessConfig ) +import qualified RIO.Process as RP ( Process, unsafeProcessHandle ) +import Stack.Prelude +import qualified System.Process as Process + +prepareForEscalation :: ProcessConfig stdin stdout stderr -> ProcessConfig stdin stdout stderr +prepareForEscalation = id + +terminateGracefully :: RP.Process stdin stdout stderr -> RIO env () +terminateGracefully p = + void $ tryAny $ liftIO $ Process.terminateProcess $ RP.unsafeProcessHandle p + +forceKill :: RP.Process stdin stdout stderr -> RIO env () +forceKill p = + void $ tryAny $ liftIO $ Process.terminateProcess $ RP.unsafeProcessHandle p diff --git a/stack.cabal b/stack.cabal index d6671e9a9d..4cfdc9f0b1 100644 --- a/stack.cabal +++ b/stack.cabal @@ -509,6 +509,7 @@ library cpp-options: -DSTACK_DISABLE_STACK_UPLOAD=False if os(windows) other-modules: + Stack.Build.TestSuiteTimeout Stack.Constants.UsrLibDirs Stack.Docker.Handlers System.Posix.User @@ -517,6 +518,7 @@ library src/windows/ else other-modules: + Stack.Build.TestSuiteTimeout Stack.Constants.UsrLibDirs Stack.Docker.Handlers System.Uname diff --git a/tests/integration/tests/6867-timeout-grace/Main.hs b/tests/integration/tests/6867-timeout-grace/Main.hs new file mode 100644 index 0000000000..c81128476a --- /dev/null +++ b/tests/integration/tests/6867-timeout-grace/Main.hs @@ -0,0 +1,38 @@ +import Data.Char (toLower) +import Data.IORef (newIORef, readIORef, writeIORef) +import Data.List (isInfixOf) +import Data.Time.Clock (diffUTCTime, getCurrentTime) +import StackTest +import Control.Monad (unless) + +main :: IO () +main = do + stack ["test", "--no-run-tests"] -- pre-build to avoid counting build time in the test + + start <- getCurrentTime + errRef <- newIORef "" + stackErrStderr + [ "test" + , "--test-suite-timeout", "1" + , "--test-suite-timeout-grace", "1" + ] + (writeIORef errRef) + end <- getCurrentTime + err <- readIORef errRef + + let errLower = map toLower err + elapsedSecs :: Double + elapsedSecs = realToFrac (diffUTCTime end start) + + logInfo $ "Elapsed time: " ++ show elapsedSecs ++ "s" + + unless ("timed out" `isInfixOf` errLower) $ + error "Expected test-suite timeout message in stderr output." + + if isWindows + then unless (elapsedSecs < 5.0) $ + error $ "Expected timeout+grace run to finish quickly on Windows, took " + ++ show elapsedSecs ++ "s" + else unless (elapsedSecs > 1.5 && elapsedSecs < 5.0) $ + error $ "Expected timeout+grace run to take about timeout+grace on Unix, took " + ++ show elapsedSecs ++ "s" diff --git a/tests/integration/tests/6867-timeout-grace/files/package.yaml b/tests/integration/tests/6867-timeout-grace/files/package.yaml new file mode 100644 index 0000000000..a0efe93896 --- /dev/null +++ b/tests/integration/tests/6867-timeout-grace/files/package.yaml @@ -0,0 +1,17 @@ +name: timeout-grace-fixture +version: 0.1.0.0 +license: BSD-3-Clause + +ghc-options: +- -Wall + +tests: + timeout-grace-fixture-test: + main: Main.hs + source-dirs: test + dependencies: + - base + when: + - condition: '!os(windows)' + dependencies: + - unix diff --git a/tests/integration/tests/6867-timeout-grace/files/stack.yaml b/tests/integration/tests/6867-timeout-grace/files/stack.yaml new file mode 100644 index 0000000000..da98263e48 --- /dev/null +++ b/tests/integration/tests/6867-timeout-grace/files/stack.yaml @@ -0,0 +1,3 @@ +snapshot: lts-24.24 +packages: +- . diff --git a/tests/integration/tests/6867-timeout-grace/files/test/Main.hs b/tests/integration/tests/6867-timeout-grace/files/test/Main.hs new file mode 100644 index 0000000000..7d2c6cf974 --- /dev/null +++ b/tests/integration/tests/6867-timeout-grace/files/test/Main.hs @@ -0,0 +1,16 @@ +{-# LANGUAGE CPP #-} + +module Main (main) where + +import Control.Concurrent (threadDelay) + +#ifndef mingw32_HOST_OS +import System.Posix.Signals (Handler (Ignore), installHandler, sigTERM) +#endif + +main :: IO () +main = do +#ifndef mingw32_HOST_OS + _ <- installHandler sigTERM Ignore Nothing +#endif + threadDelay 6000000 diff --git a/tests/integration/tests/6867-timeout-grace/files/timeout-grace-fixture.cabal b/tests/integration/tests/6867-timeout-grace/files/timeout-grace-fixture.cabal new file mode 100644 index 0000000000..4813ac2d66 --- /dev/null +++ b/tests/integration/tests/6867-timeout-grace/files/timeout-grace-fixture.cabal @@ -0,0 +1,27 @@ +cabal-version: 2.2 + +-- This file has been generated from package.yaml by hpack version 0.39.1. +-- +-- see: https://github.com/sol/hpack + +name: timeout-grace-fixture +version: 0.1.0.0 +license: BSD-3-Clause +build-type: Simple + +test-suite timeout-grace-fixture-test + type: exitcode-stdio-1.0 + main-is: Main.hs + other-modules: + Paths_timeout_grace_fixture + autogen-modules: + Paths_timeout_grace_fixture + hs-source-dirs: + test + ghc-options: -Wall + build-depends: + base + default-language: Haskell2010 + if !os(windows) + build-depends: + unix diff --git a/tests/unit/Stack/ConfigSpec.hs b/tests/unit/Stack/ConfigSpec.hs index 0f66d79f45..b64dce1df1 100644 --- a/tests/unit/Stack/ConfigSpec.hs +++ b/tests/unit/Stack/ConfigSpec.hs @@ -90,6 +90,7 @@ buildOptsConfig = " additional-args: ['-fprof']\n" ++ " coverage: true\n" ++ " no-run-tests: true\n" ++ + " test-suite-timeout-grace: 30\n" ++ " bench: true\n" ++ " benchmark-opts:\n" ++ " benchmark-arguments: -O2\n" ++ @@ -259,6 +260,7 @@ spec = beforeAll setup $ do , coverage = True , runTests = False , maximumTimeSeconds = Nothing + , timeoutGraceSeconds = Just 30 , allowStdin = True } bopts.benchmarks `shouldBe` True