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

A makefile to run doctests in Cabal-syntax #8703

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

andreasabel
Copy link
Member

make in Cabal-syntax will run the doctests there.

Couldn't get this to work in Cabal, ran into the same problems with QuickCheck as @ulysses4ever in #8504 (comment).

See:

@andreasabel andreasabel added re: devx Improving the cabal developer experience (internal issue) re: doctest Concerning doctest suites labels Jan 26, 2023
@ulysses4ever
Copy link
Collaborator

Could this be included in CI? Otherwise, I'm not sure who is the target audience of this.

@andreasabel
Copy link
Member Author

Could this be included in CI? Otherwise, I'm not sure who is the target audience of this.

The target audience is #8147 and ourselves, trying to understand how doctest is supposed to work with GHC >= 9. I had a look at the doctest job in CI and it uses GHC 8.10.7 when the world was still in order what doctest concerns.

If you think it makes sense to include this CI at this point, I can give it a shot.

However, there is quite a few open issues:

  1. This does not work for Cabal which has aprop> doctest and thus needs QuickCheck. This is likely our own fault, as the --build-depends option to cabal repl is broken in some situations (--build-depends with repl does not consume its list of inputs if it sees allow-newer #6859), and one of these situations is our Cabal subproject, as you had to discover.
  2. I'd like this to work smoothly in a multi-GHC setting where you just give the GHC_VERSION. But cabal repl -w doctest-$(GHC_VERSION) isn't working analogous to cabal repl -w ghc-$(GHC_VERSION) as it looks for ghc-pkg instead of ghc-pkg-$(GHC_VERSION). This is doctest in multi-GHC setting sol/doctest#396. Again, I am not sure whether not cabal is to be blamed here rather than doctest.
  3. We should investigate alternatives, e.g. whether cabal exec doctest is a more reliable route.
  4. I had previously used cabal-doctest in BNFC to help writing a doctest.hs testsuite that runs the doctests. However, this requires a custom-setup in the .cabal file and thus I discarded it as being too invasive.
  5. I had also previously used doctest-driver-gen but discovered problems with GHC 7 (cabal v2-test fails for doctest-driver-gen with GHC 7.10 Hexirp/doctest-driver-gen#16). Maybe this isn't a concern anymore and it could be tried again.
  6. There are other solutions I looked into, like cabal-docspec, doctest-parallel and doctest-extract. All of these restrict doctests to exported functions which I think is too limiting.

In summary, we are looking for a solution that

  • works for all packages in this repo,
  • locally and on CI,
  • with any (recent) GHC version,
  • which does not involve manually adding to a doctest.hs file each time a module gets a new doctest
  • which allows doctests on non-exported functions.

This PR is documenting a step in this direction, to save an intermediate result.

@andreasabel andreasabel marked this pull request as draft January 27, 2023 06:48
@ulysses4ever
Copy link
Collaborator

Perhaps, I'd limit the old CI job to Cabal, and add a new one for Cabal-syntax based on this makefile and using a newer GHC. Otherwise, I'm afraid this makefile will get lost and bit rot. Generally, everything that is not in CI tends to bit rot fast, in my experience.

@ulysses4ever
Copy link
Collaborator

Goes without saying that I appreciate your effort here anyways.

I'm curious: have you tried what nomeata suggested here: #8147 (comment) ? In short, don't switch to cabal repl, and specify the language Haskell2010 explicitly. I've been meaning to try it for months now but oh well... I think it could unstuck us at least w.r.t. newer GHCs.

@andreasabel
Copy link
Member Author

I'm curious: have you tried what nomeata suggested here: #8147 (comment) ?

If I understand this correctly from the code snippet, this is a different approach where you list all the modules that you want to doctest explicitly, in a testsuite file like doctest.hs. Such a module list can be generated by tools like cabal-doctest and doctest-driver-gen, but I discussed these already in my list.

@ulysses4ever
Copy link
Collaborator

@andreasabel no, my understanding is that you don't have to do anything special, just add -XHaskell2010 to the call to doctest that we already use in the CI. I just tried it with GHC 9.4.4, and it works on Cabal-syntax.

❯ doctest --fast Cabal-syntax

when making flags consistent: warning:
    Optimization flags conflict with --interactive; optimization flags ignored.

Cabal-syntax/src/Distribution/Utils/Path.hs:41:10: error:
    • Could not deduce (Typeable k1)
        arising from the superclasses of an instance declaration
      from the context: (Typeable from, Typeable to)
        bound by the instance declaration
        at Cabal-syntax/src/Distribution/Utils/Path.hs:41:10-74
    • In the instance declaration for
        ‘Structured (SymbolicPath from to)’
Examples: 130  Tried: 130  Errors: 0  Failures: 0

❯ doctest --fast Cabal-syntax -XHaskell2010

when making flags consistent: warning:
    Optimization flags conflict with --interactive; optimization flags ignored.
Examples: 130  Tried: 130  Errors: 0  Failures: 0

So, it fixes the Typeable thing. The issue is that I can't make Cabal to pass -- even with GHC 8.10.7, which is what we've been using on CI! I'm probably missing something (e.g. I'm not using cabal-env but trying cabal install --lib directly), but probably an easier thing to do is to post a draft PR and see what CI thinks. No promises though!

@ulysses4ever
Copy link
Collaborator

My little experiment is here: ulysses4ever#11

@ulysses4ever
Copy link
Collaborator

And it worked…

@andreasabel
Copy link
Member Author

And it worked…

Go for it, then!
Makes sense to switch the additional language extensions of that GHC2021 brings (and that have not been in place when the doctests were added).

@ulysses4ever
Copy link
Collaborator

@andreasabel thanks! The PR I referenced was an experiment on my own fork. The PR to the upstream is hot from the oven here #8710. You're very welcome to review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re: devx Improving the cabal developer experience (internal issue) re: doctest Concerning doctest suites
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants