Skip to content

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

Open
zlonast wants to merge 4 commits into
haskell:masterfrom
zlonast:zlonast/without-tests
Open

zlonast wants to merge 4 commits into
haskell:masterfrom
zlonast:zlonast/without-tests

Conversation

@zlonast

@zlonast zlonast commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

fix: #11858

$ 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.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

@chreekat chreekat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pointed out a few typical mistakes made by agents of the current era. There are more to be found. I suggest taking a look and trying to improve its output.

[ (CmdTest.noTestsProblem, mkTargetPackage "p-0.1")
, (CmdTest.noTestsProblem, mkTargetPackage "q-0.1")
]
do

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this test changed. The inline comment on new line 1422 doesn't explain it. At most, it makes it sound like this test has gotten worse by testing the same thing as the new test.

Comment thread changelog.d/12300.md
issues: 11858
---

`cabal test` no longer skips all tests when some of the requested targets refer

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole changelog entry is too long. Probably 10x too long in terms of lines.

Nothing
targetSelectors

let noTestsSelectors =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously the noticing or dying happened inside reportTargetProblems. I think that should continue to be the case, rather than splatting it into a higher level.

-- 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment edit is sloppy. It doesn't seem to be telling me anything important about this function in particular. It says a target is skipped with a notice, but there is no notice happening here? And there's no commit message explaining why this function changed. It's hard to understand the intent.

@chreekat

Copy link
Copy Markdown
Collaborator

Sorry to rain on your parade with this review. I do appreciate that you tackled the issue I raised! The new test looks fine; if the impl was cleaned up I'm sure this would actually fix my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"cabal test without-tests with-tests" won't run tests

2 participants