diff --git a/cabal-install/src/Distribution/Client/CmdTest.hs b/cabal-install/src/Distribution/Client/CmdTest.hs index 6ad41678849..b932c6de26c 100644 --- a/cabal-install/src/Distribution/Client/CmdTest.hs +++ b/cabal-install/src/Distribution/Client/CmdTest.hs @@ -48,8 +48,7 @@ import Distribution.Simple.Command , usageAlternatives ) import Distribution.Simple.Flag - ( Flag - , pattern Flag + ( pattern Flag ) import Distribution.Simple.Setup ( TestFlags (..) @@ -57,6 +56,7 @@ import Distribution.Simple.Setup import Distribution.Simple.Utils ( dieWithException , notice + , ordNub , warn , wrapText ) @@ -143,7 +143,7 @@ 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 @@ -151,6 +151,27 @@ testAction flags@NixStyleFlags{..} targetStrings globalFlags = do 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 diff --git a/cabal-install/tests/IntegrationTests2.hs b/cabal-install/tests/IntegrationTests2.hs index 1381b66642c..26a8704c83e 100644 --- a/cabal-install/tests/IntegrationTests2.hs +++ b/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 diff --git a/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/P.hs b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/P.hs new file mode 100644 index 00000000000..fc4877ad85e --- /dev/null +++ b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/P.hs @@ -0,0 +1 @@ +module P where diff --git a/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/Test.hs b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/Test.hs new file mode 100644 index 00000000000..d82a4bd93b7 --- /dev/null +++ b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/Test.hs @@ -0,0 +1,4 @@ +module Main where + +main :: IO () +main = return () diff --git a/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/cabal.project b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/cabal.project new file mode 100644 index 00000000000..97e14438660 --- /dev/null +++ b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/cabal.project @@ -0,0 +1 @@ +packages: ./ q/ diff --git a/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/p.cabal b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/p.cabal new file mode 100644 index 00000000000..2a6f2ad133a --- /dev/null +++ b/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 diff --git a/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/Q.hs b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/Q.hs new file mode 100644 index 00000000000..26669d9c8a8 --- /dev/null +++ b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/Q.hs @@ -0,0 +1 @@ +module Q where diff --git a/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/q.cabal b/cabal-install/tests/IntegrationTests2/targets/tests-and-no-tests/q/q.cabal new file mode 100644 index 00000000000..5255d4fdea9 --- /dev/null +++ b/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 diff --git a/changelog.d/12300.md b/changelog.d/12300.md new file mode 100644 index 00000000000..200eb543756 --- /dev/null +++ b/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.