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

doctest integration for Cabal #2327

Closed
bitemyapp opened this issue Jan 7, 2015 · 49 comments
Closed

doctest integration for Cabal #2327

bitemyapp opened this issue Jan 7, 2015 · 49 comments

Comments

@bitemyapp
Copy link
Contributor

bitemyapp commented Jan 7, 2015

New summary by @ezyang.

The goal of this task is to add a new command Setup doctest, which runs the doctest command on all of the modules of each component in a package, in much the same way Setup haddock runs haddock on all the modules of each component in a package.

There are a number of steps to implementing this:

  1. You have to add the boilerplate for setting doctest up as a new command. Grepping for occurrences of haddock should give you an idea for what you have to edit; the key files are Cabal/Distribution/Simple/Setup.hs, adding a case to defaultMainHelper in Cabal/Distribution/Simple.hs, and a new file to actually contain the logic. I'd recommend against putting too many flags into DoctestFlags when starting off.

  2. You need to add doctest to the program database, so that you can invoke it (and end-users can customize which binary to use, extra options they want to pass, etc.) The relevant logic is in Cabal/Distribution/Simple/Program/Builtin.hs You should create create something like doctestProgram.

  3. The overall logic of the driver should be something like this:

 withAllComponentsInBuildOrder pkg_descr lbi $ \component clbi -> do
   componentInitialBuildSteps (flag haddockDistPref) pkg_descr lbi clbi verbosity
   preprocessComponent pkg_descr component lbi clbi False verbosity suffixes
   -- Do the actual invocation of doctest

Between component, lbi and clbi, you will have all the information necessary to invoke doctest. You can look at https://github.com/haskell/cabal/blob/master/Cabal/Distribution/Simple/Haddock.hs for an example of all of the parts being put together.

  1. How should doctest actually be invoked? In theory, doctest accepts flag identical to GHC, so you should be able to reuse Cabal's logic for invoking GHC here. In practice, this is not true, see doctest's argument format does not match GHC's sol/doctest#155 so you will need to apply some workarounds. It might be good to get doctest to fix these discrepancies to solve problems. Still, it is probably wise to reuse the bulk of Cabal's existing logic in https://github.com/haskell/cabal/blob/master/Cabal/Distribution/Simple/GHC.hs#L509 . You may also want to consult https://github.com/phadej/cabal-doctest/blob/master/src/Distribution/Extra/Doctest.hs for "flags that doctest is known to work with". I don't necessarily recommend actually using the code from this module.

Original description below:

@sol suggested this would make more sense here than the doctest repository.

A cabal doctest or some manner of Cabal auto-magic is needed.

Should do discovery and running of doctests automatically.

It's extremely hard for people to get doctests working with a Cabal project. It took be 3 days, 10-12 hours apiece just to get doctests working for Bloodhound. Now I realize I'm a fucking moron, but I was also getting help in IRC from people like @ekmett who uses doctests for lens and he had no idea what was causing things to break either.

The things that make doctests hard to use fall into three categories:

  1. Cabal project/sandbox awareness
  2. Custom Setup.hs for doctest discovery and execution
  3. Insanely finicky doctest syntax and interactions with Haddock

This notion fixes 1 and 2, but not 3. However, if users have the reasonable belief that 1 and 2 are okay and should be working, they'll spend less time trying to diagnose the wrong things when they've messed up number 3.

I filed this issue long after having gotten Bloodhound's doctest integration working because I ran into somebody I know on IRC struggling for multiple days with this again.

I'm tired of having to tell people to copy-paste what I have in Bloodhound to make it work and people hate having to cargo-cult this anyway. Doctests have caused problems for the NICTA course as well.

@phadej
Copy link
Collaborator

phadej commented Jan 11, 2017

What I can see, than the doctest.hsc used in lens, with test-suite doctests could be directly integrated into cabal, as a "virtual component". Maybe we could add doctest-suite section if there's some configuration needed.

@ezyang
Copy link
Contributor

ezyang commented Jan 13, 2017

CC'ing @sol here.

We need to know a little more about how doctest's command line runner works before we can work something out here. I think the main question is whether or not doctest should be implemented like a virtual component (as @phadej suggests), or something more like Haddock, which is not really a component but just another mode you can run your project in.

Based on my reading of https://github.com/sol/doctest/blob/master/src/Run.hs it seems that doctests accepts exactly the same arguments that GHC accepts (have you considered making it a frontend plugin? :) https://downloads.haskell.org/~ghc/master/users-guide/extending_ghc.html#frontend-plugins ) with a little bit of extra smarts to handle options that people commonly forget to add.

My key question: does doctest recompile the module under test, or does it assume that the module has already been compiled and installed? Based on the README, it seems it must be the former. In that case, it seems like a virtual component ala @phadej is highly inappropriate, because you're going to need to also pass in all the flags that you would have needed to build the package in the first place.

Here's my question: why can't doctest be implemented as a wrapper around GHC, passed to Cabal via --with-ghc=ghc-doctest, which both (1) compiles the modules as normal, but (2) also loads and then tests the documentation examples. Because doctest needs to compile the module anyway, so it might as well slot into that codepath. Then it will get all of the package databases, etc., that it needs to load up things.

@phadej
Copy link
Collaborator

phadej commented Jan 13, 2017

I was thinking more of test-suite with type doctest. You need to be able to declare dependencies (common stanza would help to copy the ones in lib). Cabal would know the default Main.hs file, so main-is won't be required.

As far as I understand, doctest drives GHCi, so library doesn't need to be compiled, except that e.g. with bytes it's required because of the cbits

@ezyang
Copy link
Contributor

ezyang commented Jan 13, 2017

@phadej Even if the library is interpreted, we still need to pass a sufficient set of -package parameters so that all the imports go to the correct place? So, essentially, a test suite with type doctest must have exactly the same build-depends as the library, or more, otherwise you've done it wrong. That doesn't seem right.

@phadej
Copy link
Collaborator

phadej commented Jan 13, 2017

not exactly, the superset

I refine my proposal. Yes it makes more sense to have it as a virtual command:

  1. But we need to be able to add dependencies:
module MyPkg where

-- $setup
-- import MyFancyExtrasCombinators

-- | ...
--
-- >>> fancyHelperFindFixpoint fun
-- 42
fun :: Int -> Int
fun ...
name: mypkg
build-type: Simple

library
  build-depends: base ==4.9.*
  exported-modules: MyPkg

doctest foo -- do we want to support multiple setups?
  build-toos: doctest ==0.11.* -- by default just doctest
  doctest-depends: QuickCheck ==2.9.*, your-fancy-extras-combinators ==0.0.0.1
  doctest-components: mypkg:lib
  -- and all other BuildInfo flags...
  1. Cabal need to know how to compile doctest executable (there is a wrapper: https://github.com/sol/doctest/blob/master/driver/Main.hs), but it should be unique per GHC.

I stress, this is important. Cabal should be able to manage the doctest executable. Otherwise @bitemyapp won't accept this as solved (neither I). It cannot be left to the user, as it can be automated.

Related: actually Cabal should be able to manage haddock as well, as haddock is tied to single GHC as well. This isn't as urgent problem, because people rarely try to build haddocks with different GHCs.

  1. Profit.

@ezyang
Copy link
Contributor

ezyang commented Jan 13, 2017

not exactly, the superset

Sure, that's fair.

OK, so let's talk about how we actually get our hands on a doctest executable. Cabal gets away with not having to manage haddock because it's usually distributed with GHC, and when we attempt to detect ghc/ghc-pkg/etc, we also try to detect the correct Haddock for version of GHC. Haddock is tightly bound to the version of GHC in any case.

Of course, doctest isn't shipping with GHC. So what can we do? If our clients are using new-build, we're actually in pretty good shape in HEAD: all we need to do is set things up so we get an executable dependency on doctest when doctests are enabled, and new-build will go off and build doctest (against the ghc API which will be the same version as what we are compiling) and then add it to the PATH. We'll build multiple copies of doctest per version of GHC because the Nix hash will vary depending on which one we use.

I'm still not sure if we want to go the doctest stanza route; in principle, doctest can be applied to any stanza, not just library (libraries! with convenience libraries...) What if we had a magic flag that toggled doctest?

@phadej
Copy link
Collaborator

phadej commented Jan 13, 2017

Yes, I was thinking that with new-build managing doctest isn't harder than managing any other exe from build-tools.

I think stanza is clearer. Library stanza is about how to build library, not how to test it.
There could be multiple stanzas with different doctest-components (which can be multiple per stanza), as in my example.

@phadej
Copy link
Collaborator

phadej commented Jan 16, 2017

Found the package where doctest-components is essential: servant, it has some doctests in test-suite too: testing what shouldn't type-check. And these tests don't rely are informative enough to live in the lib's haddocks.

@sol
Copy link
Member

sol commented Jan 18, 2017

@ezyang Doctest does not compile anything, it uses the GHC API to extract haddock comments (parsing only) and GHCi to check the code examples. It accepts the same command-line options as ghci.

I think it should be a subcommand not a stanza for the same reasons haddock is a subcommand and not a stanza.

Contrary to popular believe, Doctest is not a tool for testing code, it's a tool for testing documentation. You want to run Doctest on exactly the same code as Haddock, to make sure that the code examples in your documentation do actually work. For that reason I think it is natural to treat it the same way as Haddock. The only difference I see is that Doctest may need QuickCheck in addition to the library dependencies.

So in general, Cabal would need to make sure that both the doctest executable and QuickCheck are available when running:

cabal doctest

For the rather uncommon case that somebody needs more additional dependencies for doctest, it may make sense to allow to specify additional dependencies somehow. But even that is not crucial from my perspective, as it is not very common.

Regarding having multiple doctest stanzas for one cabal project, I don't see how this is crucial. Specifically, using doctest to test documentation for executables is as useful (or useless for that matter) as running Haddock for executables. If we want to support this, we could add the same command-line flags we already have for cabal haddock (--executables, --tests, --benchmarks etc).

As a side note: Peoples who use Doctest for testing code (as opposed to documentation) do something fundamentally wrong. I don't think it's crucial that Cabal supports this. Other tools (e.g. Hspec) ar far superior when it comes to testing code.

@ezyang
Copy link
Contributor

ezyang commented Jan 18, 2017

Doctest does not compile anything, it uses the GHC API to extract haddock comments (parsing only) and GHCi to check the code examples.

Yes, but if we are invoking GHCi, we have to deal with essentially the same problems that we face when compiling code: what modules should we have in scope when someone asks for an import. The reason why doctests are often said to be "fragile" is because the basic usage, doctest Foo.hs, relies solely on GHCi's default package database making sense, which often won't be the case if your environment gets complicated. (Actually, package environments will help a bit with this, but let's not get into that.)

I guess this also means that doctest doesn't work on non-exposed modules or non-exported entities (because you can't actually import the module?)

So in general, Cabal would need to make sure that both the doctest executable and QuickCheck are available when running

I think the reason we are thinking about how to handle extra dependencies is not because we expect people to want this in general, but because we feel a bit uncomfortable when something like QuickCheck is hardcoded; in general we want to let people to use what they want to use, and not force them to something in particular.

I think it would be reasonable, as a first cut, to add a cabal doctest that runs the GHCi session with exactly the same packages as specified in the build-depends of the package, plus the package itself. We won't do extra dependency solving for QuickCheck, so that doctest can be run without doing dependency solving for anything extra, and solve that bridge when we get to it.

@sol
Copy link
Member

sol commented Jan 18, 2017

I guess this also means that doctest doesn't work on non-exposed modules or non-exported entities (because you can't actually import the module?)

Both work.

You have to run doctest on the sources, you don't compile the library component first. As such the concept of non-exposed modules does not exist at that point. You just invoke doctest with all the required flags and source files the same way you would run ghc.

Regarding non-exported entities, it's exactly as with ghci. You can use non-exported entities in ghci (as long as the code is interpreted). The same way you can use them in code examples that are tested with Doctest. Doctest brings the module under test into scope with :m *.

But of course yes, you still can't use non-exposed things from third-party libraries.

I think the reason we are thinking about how to handle extra dependencies is not because we expect people to want this in general, but because we feel a bit uncomfortable when something like QuickCheck is hardcoded; in general we want to let people to use what they want to use, and not force them to something in particular.

Not saying you should do something different here, but just as a clarification: QuickCheck is actually hardcoded in Doctest itself. It's used when ever you use something like

prop> (reverse . reverse) xs == (xs :: [Int])

in your Haddock comments.

Thats it from my side. Due to medical reasons I'll leave the rest to you guys. Fare well.

@phadej
Copy link
Collaborator

phadej commented Jan 18, 2017

To clarify. Yes, doctest knows about QuickCheck. But no, it needs user to tell where the QuickCheck is. Some configuration stanza is required, (in the same way haddock stanza might make sense to do flag based cpp-options for example, no body just ever asked for it).

[FL973] ~/mess % cat test.hs
module Foo (foo) where

-- | example
--
-- prop> (reverse . reverse) xs == (xs :: [Int])
foo :: String
foo = ""
[FL973] ~/mess % doctest test.hs 
### Failure in test.hs:5: expression `(reverse . reverse) xs == (xs :: [Int])'

<interactive>:38:3: error:
    Variable not in scope:
      polyQuickCheck
        :: Language.Haskell.TH.Syntax.Name -> Language.Haskell.TH.Lib.ExpQ
Examples: 1  Tried: 1  Errors: 0  Failures: 1
[FL973] ~/mess % cat test2.hs 
module Foo (foo) where

-- $setup
-- >>> import Test.QuickCheck

-- | example
--
-- prop> (reverse . reverse) xs == (xs :: [Int])
foo :: String
foo = ""
[FL973] ~/mess % doctest test2.hs 
### Failure in test2.hs:4: expression `import Test.QuickCheck'
expected: 
 but got: 
          <no location info>: error:
              Could not find module ‘Test.QuickCheck’
              It is not a module in the current program, or in any known package.
Examples: 2  Tried: 1  Errors: 0  Failures: 1
[FL973] ~/mess % doctest -package-db=$HOME/.cabal/store/ghc-8.0.1/package.db test2.hs
Examples: 2  Tried: 2  Errors: 0  Failures: 0

@23Skidoo
Copy link
Member

Re: Haddock, there was a request for adding a haddock-options field to library/executable/..., and @dcoutts was pretty strongly against it. See #926.

@alanz
Copy link
Collaborator

alanz commented Mar 23, 2017

What about generating an output file from normal GHC compilation that just has the doc-comments, in some appropriate format that a doctest tool could process, rather than processing the original source. Subject to a flag being passed to GHC.

@phadej
Copy link
Collaborator

phadej commented Mar 23, 2017

@alanz the problem is not reading the source, but providing the proper ghc flags to doctest (and where/how to configure them).

@hasufell
Copy link
Member

Ok, so doctest is still a mess to get working in 2021.

@Mikolaj
Copy link
Member

Mikolaj commented Aug 16, 2021

@hasufell: anything in particular? I've recently made my doctests work, so I thought it works fine. The trick is to do cabal build . && cabal exec cabal test doctests and not anything shorter or simpler.

@hasufell
Copy link
Member

@Mikolaj It started with trying to run it plain, which didn't work due to missing modules. Modules used in setup stage had to be added to the module itself, although that doesn't use it. Then you have to create a doctest.hs, duplicate the language extensions there as arguments, tell it about include directories etc etc

I don't consider this good UX.

@fgaz
Copy link
Member

fgaz commented Aug 16, 2021

There is also docspec

@hasufell
Copy link
Member

There is also docspec

That looks similar in complexity. I have to provide everything:

cabal-docspec \
    -w $PWD/_build/stage1/bin/ghc \
    -I $PWD/includes \
    --no-cabal-plan \
    --strip-comments \
    --timeout 2 \
    --ghci-rtsopts "-K500K" \
    --extra-package=mtl --extra-package=deepseq --extra-package=bytestring \
    -XNoImplicitPrelude \
    libraries/base/base.cabal

@fgaz
Copy link
Member

fgaz commented Aug 16, 2021

Well, that's just the "do everything manually because there's no cabal" example. In most cases, all that info will be extracted from plan.json

@Mikolaj
Copy link
Member

Mikolaj commented Aug 16, 2021

Oh yes, I forgot I had to write this monstrosity: https://github.com/LambdaHack/LambdaHack/blob/master/test/doctest-driver.hs

@gbaz
Copy link
Collaborator

gbaz commented Aug 16, 2021

I think we have improved doctest support On The Roadmap for future cabal-install. We need to nail down a good spec we're comfortable with for this, which will take some careful design work (and require someone both familiar with existing doctest machinery and cabal internals).

We should setup a new thread for discussion, but until then:

One strawman idea -- what if instead of a doctest stanza as such, we extended the test stanza with a code generation step that invoked an executable that took the code directory itself as an argument?

So we have a test stanza, with a build-tools depends on my-doctest-program and further with a field test-code-generator: my-doctest-program. This would then invoke the program (with sourcedirs as arguments) to generate some source, and then cabal would proceed to compile and test the result like normal?

@phadej
Copy link
Collaborator

phadej commented Aug 16, 2021

@hasufell

That looks similar in complexity. I have to provide everything:

You are taking the most complicated example possible, base.

My bad though, I should written the simple example first. Which is ridiculously simple for normal packages:

cabal v2-build
cabal-docspec

@phadej
Copy link
Collaborator

phadej commented Aug 16, 2021

Oh, I did. It's right there

A simplest example, which should work for most packages is to run cabal-docspec after cabal v2-build all:

cabal v2-build all
cabal-docspec

@gbaz
Copy link
Collaborator

gbaz commented Aug 16, 2021

@Mikolaj something like that, but instead of needing to pass all the stuff in a directive in a stub driver file, one would simply give the executable name directly as clause in the test stanza.

So something like

  main-is:            doctest-gen/Main.hs
  build-tool-depends: doctest-driver-gen:doctest-driver-gen
  test-code-generator: doctest-driver-gen

and nothing else.

@hasufell
Copy link
Member

So, I get 3 different results with 3 tools:

  1. stack:
abstract-filepath> Test suite abstract-filepath-test passed
abstract-filepath> test (suite: doctest)

Examples: 368  Tried: 65  Errors: 0  Failures: 0^C⏎  
  1. cabal
lib/AFP/Data/ByteString/Short.hs:171:1: error:
    Could not load module ‘Data.DList’
    It is a member of the hidden package ‘dlist-1.0’.
    You can run ‘:set -package dlist’ to expose it.
    (Note: this unloads all the modules in the current scope.)
    It is a member of the hidden package ‘dlist-1.0’.
    You can run ‘:set -package dlist’ to expose it.
    (Note: this unloads all the modules in the current scope.)
    It is a member of the hidden package ‘dlist-1.0’.
    You can run ‘:set -package dlist’ to expose it.
    (Note: this unloads all the modules in the current scope.)
    It is a member of the hidden package ‘dlist-0.8.0.8’.
    You can run ‘:set -package dlist’ to expose it.
    (Note: this unloads all the modules in the current scope.)
    It is a member of the hidden package ‘dlist-0.8.0.7’.
    You can run ‘:set -package dlist’ to expose it.
    (Note: this unloads all the modules in the current scope.)
    it is a hidden module in the package ‘HsYAML-0.2.1.0’
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
    |
171 | import qualified Data.DList as DL
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  1. cabal-docspec
[   0.40935] docspec.phase2: AFP.AbstractFilePath.Internal.Posix
[   0.43823] error: import Test.QuickCheck
[   0.43844] error: in comment at 109:1
expected:
 but got:
          ^
          <no location info>: error:
              Could not load module ‘Test.QuickCheck’
              It is a member of the hidden package ‘QuickCheck-2.14.2’.
              You can run ‘:set -package QuickCheck’ to expose it.
              (Note: this unloads all the modules in the current scope.)
              It is a member of the hidden package ‘QuickCheck-2.14.2’.
              You can run ‘:set -package QuickCheck’ to expose it.
              (Note: this unloads all the modules in the current scope.)
              It is a member of the hidden package ‘QuickCheck-2.14.2’.
              You can run ‘:set -package QuickCheck’ to expose it.
              (Note: this unloads all the modules in the current scope.)
              It is a member of the hidden package ‘QuickCheck-2.13.2’.
              You can run ‘:set -package QuickCheck’ to expose it.
              (Note: this unloads all the modules in the current scope.)
              It is a member of the hidden package ‘QuickCheck-2.13.2’.
              You can run ‘:set -package QuickCheck’ to expose it.
              (Note: this unloads all the modules in the current scope.)

[   0.44166] warning[-Werror-in-setup]: Issue in $setup, skipping AFP.AbstractFilePath.Internal.Posix module
[   0.44206] docspec.phase2: AFP.AbstractFilePath.Internal.Windows
[   0.46181] error: import Test.QuickCheck
[   0.46202] error: in comment at 90:1

@phadej
Copy link
Collaborator

phadej commented Aug 19, 2021

With following straight forward diff:

diff --git a/abstract-filepath.cabal b/abstract-filepath.cabal
index 942767b..bd3482f 100644
--- a/abstract-filepath.cabal
+++ b/abstract-filepath.cabal
@@ -52,6 +52,8 @@ library
     , text              ^>=1.2.4.1
     , word8             ^>=0.1
 
+  x-docspec-extra-packages: QuickCheck
+
 test-suite abstract-filepath-test
   type:               exitcode-stdio-1.0
   main-is:            Test.hs
diff --git a/lib/AFP/AbstractFilePath/Common.hs b/lib/AFP/AbstractFilePath/Common.hs
index 665ea30..fbc5f01 100644
--- a/lib/AFP/AbstractFilePath/Common.hs
+++ b/lib/AFP/AbstractFilePath/Common.hs
@@ -152,6 +152,7 @@ import qualified AFP.Data.ByteString.Short as BS
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Test.QuickCheck
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types
@@ -170,6 +171,7 @@ import qualified AFP.Data.ByteString.Short as BS
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Test.QuickCheck
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types
diff --git a/lib/AFP/AbstractFilePath/Internal/Common.hs b/lib/AFP/AbstractFilePath/Internal/Common.hs
index ae93f01..b2aa02b 100644
--- a/lib/AFP/AbstractFilePath/Internal/Common.hs
+++ b/lib/AFP/AbstractFilePath/Internal/Common.hs
@@ -88,10 +88,12 @@ type Word = Word8
 
 #ifdef WINDOWS
 -- $setup
+-- >>> import Prelude hiding (Word)
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
 -- >>> import Test.QuickCheck
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types (PosixFilePath (..))
 -- >>> import AFP.OsString.Internal.Types
@@ -107,10 +109,13 @@ type Word = Word8
 -- >>> let _chr :: Word -> Char; _chr = chr . fromIntegral
 #else
 -- $setup
+-- >>> :set -XFlexibleInstances
+-- >>> import Prelude hiding (Word)
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
 -- >>> import Test.QuickCheck
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types (PosixFilePath (..))
 -- >>> import AFP.OsString.Internal.Types
[polinukli] /codetmp/abstract-filepath master % git diff
diff --git a/abstract-filepath.cabal b/abstract-filepath.cabal
index 942767b..bd3482f 100644
--- a/abstract-filepath.cabal
+++ b/abstract-filepath.cabal
@@ -52,6 +52,8 @@ library
     , text              ^>=1.2.4.1
     , word8             ^>=0.1
 
+  x-docspec-extra-packages: QuickCheck
+
 test-suite abstract-filepath-test
   type:               exitcode-stdio-1.0
   main-is:            Test.hs
diff --git a/lib/AFP/AbstractFilePath/Common.hs b/lib/AFP/AbstractFilePath/Common.hs
index 665ea30..fbc5f01 100644
--- a/lib/AFP/AbstractFilePath/Common.hs
+++ b/lib/AFP/AbstractFilePath/Common.hs
@@ -152,6 +152,7 @@ import qualified AFP.Data.ByteString.Short as BS
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Test.QuickCheck
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types
@@ -170,6 +171,7 @@ import qualified AFP.Data.ByteString.Short as BS
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Test.QuickCheck
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types
diff --git a/lib/AFP/AbstractFilePath/Internal/Common.hs b/lib/AFP/AbstractFilePath/Internal/Common.hs
index ae93f01..b2aa02b 100644
--- a/lib/AFP/AbstractFilePath/Internal/Common.hs
+++ b/lib/AFP/AbstractFilePath/Internal/Common.hs
@@ -88,10 +88,12 @@ type Word = Word8
 
 #ifdef WINDOWS
 -- $setup
+-- >>> import Prelude hiding (Word)
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
 -- >>> import Test.QuickCheck
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types (PosixFilePath (..))
 -- >>> import AFP.OsString.Internal.Types
@@ -107,10 +109,13 @@ type Word = Word8
 -- >>> let _chr :: Word -> Char; _chr = chr . fromIntegral
 #else
 -- $setup
+-- >>> :set -XFlexibleInstances
+-- >>> import Prelude hiding (Word)
 -- >>> import Data.Char
 -- >>> import Data.Maybe
 -- >>> import Data.Word8
 -- >>> import Test.QuickCheck
+-- >>> import Data.ByteString.Short (ShortByteString)
 -- >>> import Control.Applicative
 -- >>> import AFP.AbstractFilePath.Internal.Types (PosixFilePath (..))
 -- >>> import AFP.OsString.Internal.Types

the cabal-docspec runs plenty:

[   0.81693] error: \n ->  (_chr n == '/') == isPathSeparator' (BS.singleton n)
[   0.81735] error: in comment at 168:1

<interactive>:134:57: error: Not in scope: ‘BS.singleton’

[   0.87951] docspec.phase2: AFP.AbstractFilePath.Internal.Windows
[   1.18531] docspec.phase2: AFP.AbstractFilePath.Posix
[   1.26488] error: instance Arbitrary PosixFilePath where arbitrary = PS <$> arbitrary
[   1.26532] error: in comment at 170:1
expected: 
 but got: 
          ^
          <interactive>:422:52: error:
              • Data constructor not in scope: PS :: a0 -> PosixFilePath
              • Perhaps you meant variable ‘_S’ (imported from Data.Word8)

[   1.26638] warning[-Werror-in-setup]: Issue in $setup, skipping AFP.AbstractFilePath.Posix module
[   1.26690] docspec.phase2: AFP.AbstractFilePath.Windows
[   1.31394] error: instance Arbitrary WindowsFilePath where arbitrary = WS <$> arbitrary
[   1.31439] error: in comment at 151:1
expected: 
 but got: 
          ^
          <interactive>:450:54: error:
              • Data constructor not in scope: WS :: a0 -> WindowsFilePath
              • Perhaps you meant variable ‘_S’ (imported from Data.Word8)

[   1.31554] warning[-Werror-in-setup]: Issue in $setup, skipping AFP.AbstractFilePath.Windows module
[   1.40797] doctest.summary: 
Total:       382; Tried:   67; Skipped:  315; Success:   64; Errors:    3; Failures    0
Examples:    259; Tried:    0; Skipped:  259; Success:    0; Errors:    0; Failures    0
Properties:   53; Tried:    7; Skipped:   46; Success:    6; Errors:    1; Failures    0
Setup:        70; Tried:   60; Skipped:   10; Success:   58; Errors:    2; Failures    0

[   1.40973] error: there were errors or property failures

The extra cabal field is documented: https://github.com/phadej/cabal-extras/blob/master/cabal-docspec/MANUAL.md#cabal-fields as well as reasoning for not importing modules https://github.com/phadej/cabal-extras/blob/master/cabal-docspec/MANUAL.md#q-why-cabal-docspec-doesnt-import-modules-automatically

I think that three errors left are also fixed easily with adding more imports to $setup blocks.

Yes, it's not "out of the box - just works", but it works with acceptable amount of extra work, I doubt Cabal based solution would work without any extra config & tweaks either.

Obviously, I'd like approach like cabal-docspec integrated into Cabal. It works for kmettverse and my libs. Sadly it doesn't use the doctest lib, but IMO its approach is not perfect, and there is no need to stick to it for any cost.

@hasufell
Copy link
Member

@phadej I tried fixing all the imports and now cabal-docspec throws a lot of errors that stack doesn't, which suggests CPP is handled differently:

   1.21978] warning[-Wskipped-property]: in comment at 89:1 skipping \path -> CTOR (uncurry (\(CTOR a) (CTOR b) -> BS.append a b) (splitExtension path)) == path
[   1.22479] error: splitExtension "file.exe"
[   1.22498] error: in comment at 89:1
expected: ("file",".exe")
 but got:
          ^
          <interactive>:456:16: error:
              • Couldn't match type ‘[Char]’ with ‘PosixString’
                Expected type: PosixFilePath
                  Actual type: [Char]
              • In the first argument of ‘splitExtension’, namely ‘"file.exe"’
                In the expression: splitExtension "file.exe"
                In an equation for ‘it’: it = splitExtension "file.exe"

Branch with the code: https://github.com/hasufell/abstract-filepath/tree/doctest

@phadej
Copy link
Collaborator

phadej commented Aug 19, 2021

Yes, the extensions in files are not enabled either. https://github.com/phadej/cabal-extras/blob/master/cabal-docspec/MANUAL.md#using-ghc-extensions

@hasufell
Copy link
Member

Yes, the extensions in files are not enabled either. https://github.com/phadej/cabal-extras/blob/master/cabal-docspec/MANUAL.md#using-ghc-extensions

I don't understand, what extension am I supposed to pass?

Even cabal-docspec -XCPP -XTypeSynonymInstances doesn't work.

@phadej
Copy link
Collaborator

phadej commented Aug 19, 2021

Couldn't match type ‘[Char]’ with ‘PosixString’

Looks very much like you need -XOverloadedStrings

@hasufell
Copy link
Member

@phadej that finally worked. But can you feel my pain? Now I have to add special sections into my cabal file and still need to pass flags on the command line. Since cabal-docspec isn't on hackage I also can't easily add it to build-tool-depends and users of my library need to install it manually before they can run the test.

@phadej
Copy link
Collaborator

phadej commented Aug 19, 2021

You can specify the command line parameters inside the .cabal file, https://github.com/phadej/cabal-extras/blob/master/cabal-docspec/MANUAL.md#cabal-fields, or pass --extra-package on the command line. Former is usually nicer as then cabal-doctest should just work.

The cabal-docspec not being on Hackage is my deliberate choice. The binaries are there, not hard to download them (if someone asked nicely, I could provide windows and osx binaries too). build-tool-dependswon't work, it's as horrible hack as runninghlintas atest-suite. (Also, it's a lot faster to download hlint binary then compile its dependencies, cabal-docspec` isn't dependency light either).

I also don't yet see any conceptual challenges why cabal couldn't just copy what cabal-docspec does. so instead of cabal-docspec you'd say cabal docspec. That design seems to work, and it is documented.

@fgaz
Copy link
Member

fgaz commented Aug 19, 2021

So let's reopen the ticket i guess?

@phadej
Copy link
Collaborator

phadej commented Aug 19, 2021

cabal-docspec works on the level of cabal-install, not Setup.hs. Whether Setup.sh doctest command should exist, and how it would work is unclear. One argument is that there aren't Setup.hs hlint, yet the counterargument is "why not".

I'd open a ticket only when there is some concrete plan, there are discussions for getting consensus (and actually RFCs).

@phadej
Copy link
Collaborator

phadej commented Aug 19, 2021

Another point is that there's stan, hlint is not unique. doctest and cabal-docspec are different. There are also source formatters. Whether and how they all should integrate into cabal-install?

For example, I considered making a variant of cabal-docspec which reads its inputs from external files. I.e. works more as actual tests, not as "examples in the documentation". Separate files could be more convenient in some situations.

That's an argument of not integrating anything too closely with cabal-install, but rather make running more convenient.

This reminds me that I should make cabal-hlint which would configure hlint based on the .cabal file contents (i.e. default-extensions), and run it on all packages in the project.

@hasufell
Copy link
Member

hasufell commented Aug 19, 2021

Have to note that only stack and cabal-docspec work. cabal test still throws (code):

Test suite doctest: RUNNING...

lib/AFP/Data/ByteString/Short.hs:159:1: error:
    Could not find module ‘Data.Word8’
    Perhaps you meant
      Data.Word (from base-4.14.3.0)
      Data.Ord (from base-4.14.3.0)
      Data.Void (from base-4.14.3.0)
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
    |
159 | import Data.Word8
    | ^^^^^^^^^^^^^^^^^

lib/AFP/Data/ByteString/Short.hs:170:1: error:
    Could not find module ‘Data.Primitive.ByteArray’
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
    |
170 | import qualified Data.Primitive.ByteArray as BA
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Test suite doctest: FAIL
Test suite logged to:
/home/hasufell/git/abstract-filepath/dist-newstyle/build/x86_64-linux/ghc-8.10.6/abstract-filepath-0.1.0.0/t/doctest/test/abstract-filepath-0.1.0.0-doctest.log
0 of 1 test suites (0 of 1 test cases) passed.
cabal: Tests failed for test:doctest from abstract-filepath-0.1.0.0.

At this point, most users would have given up. So my impression is that it's a problem with cabal test, especially since stack test works.

@gbaz
Copy link
Collaborator

gbaz commented Aug 19, 2021

I would advocate not reopening the ticket. We have a separate ticket for cabal v2-doctest: #4500

@gbaz
Copy link
Collaborator

gbaz commented Aug 19, 2021

(also @hasufell it looks like to get cabal test to work one needs to add the appropriate packages to the build-depends of the test doctest stanza, which seems eminently reasonable to me?)

@hasufell
Copy link
Member

(also @hasufell it looks like to get cabal test to work one needs to add the appropriate packages to the build-depends of the test doctest stanza, which seems eminently reasonable to me?)

That doesn't fix it.

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

No branches or pull requests