Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testsuite: Be explicit about runtime test dependencies #9439

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

mpickering
Copy link
Collaborator

Issue #8356 reports occasional errors from running the testsuite about multiple package versions available. This stems from the invokation of runghc not being explicit about all dependencies of the testsuite.

The solution is provide a component in the cabal file which is explicit about which packages the tests can depend on. This component has a build-depends section which lists all the dependencies that the tests require.

It would be better if this component was a library component but we can't do this with a Custom setup because of limitations to do with per-component builds.

Then we also enable -hide-all-packages, so the dependency will not be available if it is not explicitly listed as a dependency.

You could also imagine a future where the Setup.hs script found the test files and compiled a single executable which would run all the tests, rather than invoking runghc on each one individually.

Fixes #8356

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

That's a neat solution! LGTM

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

Fingers crossed it finally fixes it!

@mpickering mpickering added the merge me Tell Mergify Bot to merge label Nov 13, 2023
@ulysses4ever ulysses4ever mentioned this pull request Nov 13, 2023
1 task
Copy link
Collaborator

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

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

👍

cabal-testsuite/cabal-testsuite.cabal Outdated Show resolved Hide resolved
@mpickering mpickering added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Nov 16, 2023
@Mikolaj
Copy link
Member

Mikolaj commented Nov 16, 2023

We tend to make a bit of ceremony when applying delay_passed, for transparency and mutual trust and to make sure newcomers don't see old PRs and try to emulate our crooked ways. IMHO in this case emergency merge is justified and, by writing this, I hope I fulfilled the ritual. ;) Still time to object and remove the label, though, while the PR is in the queue (and will probably get rebased again).

Issue haskell#8356 reports occasional errors from running the testsuite about
multiple package versions available. This stems from the invokation of
`runghc` not being explicit about all dependencies of the testsuite.

The solution is provide a component in the cabal file which is explicit
about which packages the tests can depend on. This component has a
build-depends section which lists all the dependencies that the tests
require.

It would be better if this component was a library component but we
can't do this with a Custom setup because of limitations to do with
per-component builds.

Then we also enable `-hide-all-packages`, so the dependency will not be
available if it is not explicitly listed as a dependency.

You could also imagine a future where the Setup.hs script found the test
files and compiled a single executable which would run all the tests,
rather than invoking runghc on each one individually.

Fixes haskell#8356
@mergify mergify bot merged commit 27f4ee7 into haskell:master Nov 16, 2023
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: cabal-testsuite tests fail with "double directory"
6 participants