Merge pull request #6645 from commercialhaskell/fix6643

Fix #6643 Respect `--no-run-tests`, `--no-run-benchmarks` when listing actions

ChangeLog.md

@@ -21,6 +21,10 @@ Behavior changes:
  character in Stack's 'programs' path, as GHC 9.4.1 and later do not work if
  there is a space in the path to the `ghc` executable. S-8432 now presents as a
  warning and not an error.
+* Stack respects the `--no-run-tests` and `--no-run-benchmarks` flags when
+ determining build actions. Previously Stack respected the flags when executing
+ the run test suites or run benchmarks actions for each targeted project
+ package.
 
 Other enhancements:
 

doc/maintainers/stack_errors.md

@@ -5,7 +5,7 @@
 In connection with considering Stack's support of the
 [Haskell Error Index](https://errors.haskell.org/) initiative, this page seeks
 to take stock of the errors that Stack itself can raise, by reference to the
-`master` branch of the Stack repository. Last updated: 2024-08-02.
+`master` branch of the Stack repository. Last updated: 2024-09-07.
 
 * `Stack.main`: catches exceptions from action `commandLineHandler`.
 
@@ -385,6 +385,7 @@ to take stock of the errors that Stack itself can raise, by reference to the
  [S-8100] | GHCProfOptionInvalid
  [S-1727] | NotOnlyLocal [PackageName] [Text]
  [S-6362] | CompilerVersionMismatch (Maybe (ActualCompiler, Arch)) (WantedCompiler, Arch) GHCVariant CompilerBuild VersionCheck WantedCompilerSetter Text
+ [S-4660] | ActionNotFilteredBug StyleDoc
  ~~~
 
  - `Stack.Types.Compiler.CompilerException`

src/Stack/Build/Execute.hs

@@ -367,8 +367,18 @@ executePlan' :: HasEnvConfig env
  -> ExecuteEnv
  -> RIO env ()
 executePlan' installedMap0 targets plan ee = do
+ config <- view configL
  let !buildOpts = ee.buildOpts
  !testOpts = buildOpts.testOpts
+ !benchmarkOpts = buildOpts.benchmarkOpts
+ runTests = testOpts.runTests
+ runBenchmarks = benchmarkOpts.runBenchmarks
+ noNotifyIfNoRunTests = not config.notifyIfNoRunTests
+ noNotifyIfNoRunBenchmarks = not config.notifyIfNoRunBenchmarks
+ hasTests = not . Set.null . testComponents . taskComponents
+ hasBenches = not . Set.null . benchComponents . taskComponents
+ tests = Map.elems $ Map.filter hasTests plan.finals
+ benches = Map.elems $ Map.filter hasBenches plan.finals
  when testOpts.coverage deleteHpcReports
  cv <- view actualCompilerVersionL
  whenJust (nonEmpty $ Map.toList plan.unregisterLocal) $ \ids -> do
@@ -400,6 +410,24 @@ executePlan' installedMap0 targets plan ee = do
  buildOpts.keepGoing
  terminal <- view terminalL
  terminalWidth <- view termWidthL
+ unless (noNotifyIfNoRunTests || runTests || null tests) $
+ prettyInfo $
+ fillSep
+ [ flow "All test running disabled by"
+ , style Shell "--no-run-tests"
+ , flow "flag. To mute this message in future, set"
+ , style Shell (flow "notify-if-no-run-tests: false")
+ , flow "in Stack's configuration."
+ ]
+ unless (noNotifyIfNoRunBenchmarks || runBenchmarks || null benches) $
+ prettyInfo $
+ fillSep
+ [ flow "All benchmark running disabled by"
+ , style Shell "--no-run-benchmarks"
+ , flow "flag. To mute this message in future, set"
+ , style Shell (flow "notify-if-no-run-benchmarks: false")
+ , flow "in Stack's configuration."
+ ]
  errs <- liftIO $ runActions threads keepGoing actions $
  \doneVar actionsVar -> do
  let total = length actions
@@ -564,8 +592,9 @@ toActions installedMap mtestLock runInBase ee (mbuild, mfinal) =
  , concurrency = ConcurrencyAllowed
  }
  ) $
- -- These are the "final" actions - running tests and benchmarks.
- ( if Set.null tests
+ -- These are the "final" actions - running test suites and benchmarks,
+ -- unless --no-run-tests or --no-run-benchmarks is enabled.
+ ( if Set.null tests || not runTests
  then id
  else (:) Action
  { actionId = ActionId pkgId ATRunTests
@@ -578,7 +607,7 @@ toActions installedMap mtestLock runInBase ee (mbuild, mfinal) =
  , concurrency = ConcurrencyAllowed
  }
  ) $
- ( if Set.null benches
+ ( if Set.null benches || not runBenchmarks
  then id
  else (:) Action
  { actionId = ActionId pkgId ATRunBenchmarks
@@ -615,6 +644,8 @@ toActions installedMap mtestLock runInBase ee (mbuild, mfinal) =
  bopts = ee.buildOpts
  topts = bopts.testOpts
  beopts = bopts.benchmarkOpts
+ runTests = topts.runTests
+ runBenchmarks = beopts.runBenchmarks
 
 taskComponents :: Task -> Set NamedComponent
 taskComponents task =

src/Stack/Build/ExecutePackage.hs

@@ -996,12 +996,7 @@ singleTest topts testsToRun ac ee task installedMap = do
  announce "rerunning previously failed test"
  pure True
  TSUnknown -> pure True
- else do
- notifyIfNoRunTests <- view $ configL . to (.notifyIfNoRunTests)
- when notifyIfNoRunTests $
- announce "Test running disabled by --no-run-tests flag."
- pure False
-
+ else prettyThrowM $ ActionNotFilteredBug "singleTest"
  when toRun $ do
  buildDir <- distDirFromDir pkgDir
  hpcDir <- hpcDirFromDir pkgDir
@@ -1262,17 +1257,10 @@ singleBench beopts benchesToRun ac ee task installedMap = do
  let args = map unqualCompToString benchesToRun <> maybe []
  ((:[]) . ("--benchmark-options=" <>))
  beopts.additionalArgs
-
  toRun <-
  if beopts.runBenchmarks
  then pure True
- else do
- notifyIfNoRunBenchmarks <-
- view $ configL . to (.notifyIfNoRunBenchmarks)
- when notifyIfNoRunBenchmarks $
- announce "Benchmark running disabled by --no-run-benchmarks flag."
- pure False
-
+ else prettyThrowM $ ActionNotFilteredBug "singleBench"
  when toRun $ do
  announce "benchmarks"
  cabal CloseOnException KeepTHLoading ("bench" : args)

src/Stack/Types/Build/Exception.hs

@@ -261,6 +261,7 @@ data BuildPrettyException
  VersionCheck
  WantedCompilerSetter -- Way that the wanted compiler is set
  StyleDoc -- recommended resolution
+ | ActionNotFilteredBug StyleDoc
  deriving (Show, Typeable)
 
 instance Pretty BuildPrettyException where
@@ -450,6 +451,12 @@ instance Pretty BuildPrettyException where
  ]
  <> blankLine
  <> resolution
+ pretty (ActionNotFilteredBug source) = bugPrettyReport "S-4660" $
+ fillSep
+ [ source
+ , flow "is seeking to run an action that should have been filtered from \
+ \the list of actions."
+ ]
 
 instance Exception BuildPrettyException
 

tests/integration/tests/3959-order-of-flags/Main.hs

@@ -17,5 +17,5 @@ checkFlagsAfterCommand = stackCheckStderr ["build", "--test", "--no-run-tests"]
 
 checker :: String -> IO ()
 checker output = do
- let testsAreDisabled = any (\ln -> "Test running disabled by" `isInfixOf` ln) (lines output)
+ let testsAreDisabled = any (\ln -> "All test running disabled by" `isInfixOf` ln) (lines output)
  unless testsAreDisabled $ fail "Tests should not be run"