fix(cabal-install): cabal test runs the tests of the targets that have them

cabal-install/src/Distribution/Client/CmdTest.hs

@@ -48,15 +48,15 @@ import Distribution.Simple.Command
   , usageAlternatives
   )
 import Distribution.Simple.Flag
-  ( Flag
-  , pattern Flag
+  ( pattern Flag
   )
 import Distribution.Simple.Setup
   ( TestFlags (..)
   )
 import Distribution.Simple.Utils
   ( dieWithException
   , notice
+  , ordNub
   , warn
   , wrapText
   )
@@ -143,14 +143,35 @@ testAction flags@NixStyleFlags{..} targetStrings globalFlags = do
       -- Interpret the targets on the command line as test targets
       -- (as opposed to say build or haddock targets).
       targets <-
-        either (reportTargetProblems verbosity failWhenNoTestSuites) return $
+        either (reportTargetProblems verbosity) return $
           resolveTargetsFromSolver
             selectPackageTargets
             selectComponentTarget
             elaboratedPlan
             Nothing
             targetSelectors
 
+      let noTestsSelectors =
+            ordNub
+              ( filter
+                  (`notElem` uniqueTargetSelectors targets)
+                  targetSelectors
+              )
+
+      case noTestsSelectors of
+        [] -> return ()
+        _ -> case failWhenNoTestSuites of
+          Flag True ->
+            dieWithException verbosity $
+              ReportTargetProblems
+                ( unlines
+                    (map (renderTestTargetProblem . noTestsProblem) noTestsSelectors)
+                )
+          _ -> do
+            for_ noTestsSelectors $ \selector ->
+              notice verbosity (renderAllowedNoTestsProblem selector)
+            when (null (allTargetSelectors targets)) System.Exit.exitSuccess
+
       let elaboratedPlan' =
             pruneInstallPlanToTargets
               TargetActionTest
@@ -171,8 +192,11 @@ testAction flags@NixStyleFlags{..} targetStrings globalFlags = do
 -- It selects the 'AvailableTarget's that the 'TargetSelector' refers to,
 -- or otherwise classifies the problem.
 --
--- For the @test@ command we select all buildable test-suites,
--- or fail if there are no test-suites or no buildable test-suites.
+-- For the @test@ command we select all buildable test-suites.
+-- A target that contains no test-suites does not select anything: it is
+-- skipped with a notice instead of aborting the command (see #11858), so
+-- we only report a problem if there are test-suites but none are buildable,
+-- or if there is nothing to select a test-suite from at all.
 selectPackageTargets
   :: TargetSelector
   -> [AvailableTarget k]
@@ -184,12 +208,12 @@ selectPackageTargets targetSelector targets
   -- If there are test-suites but none are buildable then we report those
   | not (null targetsTests) =
       Left (TargetProblemNoneEnabled targetSelector targetsTests)
-  -- If there are no test-suite but some other targets then we report that
-  | not (null targets) =
-      Left (noTestsProblem targetSelector)
   -- If there are no targets at all then we report that
-  | otherwise =
+  | null targets =
       Left (TargetProblemNoTargets targetSelector)
+  -- If there are no test-suites then there is nothing to select
+  | otherwise =
+      Right []
   where
     targetsTestsBuildable =
       selectBuildableTargets
@@ -255,22 +279,13 @@ isSubComponentProblem pkgid name subcomponent =
   CustomTargetProblem $
     TargetProblemIsSubComponent pkgid name subcomponent
 
-reportTargetProblems :: Verbosity -> Flag Bool -> [TestTargetProblem] -> IO a
-reportTargetProblems verbosity failWhenNoTestSuites problems =
-  case (failWhenNoTestSuites, problems) of
-    (Flag True, [CustomTargetProblem (TargetProblemNoTests _)]) ->
-      dieWithException verbosity $ ReportTargetProblems problemsMessage
-    (_, [CustomTargetProblem (TargetProblemNoTests selector)]) -> do
-      notice verbosity (renderAllowedNoTestsProblem selector)
-      System.Exit.exitSuccess
-    (_, _) -> dieWithException verbosity $ ReportTargetProblems problemsMessage
-  where
-    problemsMessage = unlines . map renderTestTargetProblem $ problems
+reportTargetProblems :: Verbosity -> [TestTargetProblem] -> IO a
+reportTargetProblems verbosity =
+  dieWithException verbosity
+    . ReportTargetProblems
+    . unlines
+    . map renderTestTargetProblem
 
--- | Unless @--test-fail-when-no-test-suites@ flag is passed, we don't
---   @die@ when the target problem is 'TargetProblemNoTests'.
---   Instead, we display a notice saying that no tests have run and
---   indicate how this behaviour was enabled.
 renderAllowedNoTestsProblem :: TargetSelector -> String
 renderAllowedNoTestsProblem selector =
   "No tests to run for " ++ renderTargetSelector selector

cabal-install/tests/IntegrationTests2.hs

@@ -1417,14 +1417,34 @@ testTargetProblemsTest config reportSubCase = do
     ]
 
   reportSubCase "no tests"
-  assertProjectTargetProblems
-    "targets/simple"
-    config
-    CmdTest.selectPackageTargets
-    CmdTest.selectComponentTarget
-    [ (CmdTest.noTestsProblem, mkTargetPackage "p-0.1")
-    , (CmdTest.noTestsProblem, mkTargetPackage "q-0.1")
-    ]
+  do
+    (_, elaboratedPlan, _) <- planProject "targets/simple" config
+    -- Packages without test suites select no targets: they are skipped with
+    -- a notice instead of aborting the command (see #11858).
+    assertProjectDistinctTargets
+      elaboratedPlan
+      CmdTest.selectPackageTargets
+      CmdTest.selectComponentTarget
+      [mkTargetPackage "p-0.1"]
+      []
+    assertProjectDistinctTargets
+      elaboratedPlan
+      CmdTest.selectPackageTargets
+      CmdTest.selectComponentTarget
+      [mkTargetPackage "p-0.1", mkTargetPackage "q-0.1"]
+      []
+
+  reportSubCase "pkg with tests and pkg without tests"
+  do
+    (_, elaboratedPlan, _) <- planProject "targets/tests-and-no-tests" config
+    -- The package with test suites must still be selected when it is
+    -- requested together with a package without test suites (see #11858).
+    assertProjectDistinctTargets
+      elaboratedPlan
+      CmdTest.selectPackageTargets
+      CmdTest.selectComponentTarget
+      [mkTargetPackage "p-0.1", mkTargetPackage "q-0.1"]
+      [("p-0.1-inplace-p-tests", CTestName "p-tests")]
 
   reportSubCase "not a test"
   assertProjectTargetProblems

cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/P.hs

@@ -0,0 +1 @@
+module P where

cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/Test.hs

@@ -0,0 +1,4 @@
+module Main where
+
+main :: IO ()
+main = return ()

cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/cabal.project

@@ -0,0 +1 @@
+packages: ./ q/

cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/p.cabal

@@ -0,0 +1,13 @@
+cabal-version: 3.8
+name: p
+version: 0.1
+build-type: Simple
+
+library
+  exposed-modules: P
+  build-depends: base
+
+test-suite p-tests
+  type: exitcode-stdio-1.0
+  main-is: Test.hs
+  build-depends: base

cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/Q.hs

@@ -0,0 +1 @@
+module Q where

cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/q.cabal

@@ -0,0 +1,8 @@
+cabal-version: 3.8
+name: q
+version: 0.1
+build-type: Simple
+
+library
+  exposed-modules: Q
+  build-depends: base

changelog.d/12300.md

@@ -0,0 +1,28 @@
+---
+synopsis: "`cabal test` runs the tests of the targets that have them"
+packages: [cabal-install]
+prs: 12300
+issues: 11858
+---
+
+`cabal test` no longer skips all tests when some of the requested targets refer
+to packages that do not contain any test suites. Previously, one such target
+would abort the whole command with a notice and a successful exit status,
+silently skipping the test suites of all other targets — a foot-gun in CI
+environments, since failing tests could go unnoticed.
+
+Now each target without test suites is skipped with a notice, and the test
+suites of the remaining targets are built and run as usual:
+
+```pre
+$ cabal test pkg-with-tests pkg-without-tests
+No tests to run for the package pkg-without-tests-0.1.0.0
+Running 1 test suites...
+Test suite pkg-with-tests-test: PASS
+1 of 1 test suites (1 of 1 test cases) passed.
+```
+
+The exit status is now determined by the outcome of the tests that ran.
+As before, `cabal test` exits successfully when no requested target contains
+test suites, unless `--test-fail-when-no-test-suites` is passed, in which case
+targets without test suites remain an error.