Run some of Haddock's tests in the testsuite

The 4 main testsuites in Haddock don't have many dependencies, but are regularly broken in small ways by changes to the GHC AST or the GHC API.

This adds four tests to the testsuite that build the four haddock suites (html-test, latex-test, hoogle-test, and hypsrc-test) and run to check that nothing fails in them.

Fixes #16206 (closed)

mentioned in issue #16264 (closed)

Is this something that would be merge-able? It would certainly save a lot of pain around GHC regularly breaking Haddock...

If it is something we can do:

• how do we add a new submodule for xml? CI seems unhappy...
• I'll fix the Make system too (current changes only work for Hadrian)
• I'll bump the Haddock submodule separately

changed the description

@harpocrates The Hadrian part looks good to me, thank you!

added 1 commit

• ee827ced - Fix the Make system

Compare with previous version

unmarked as a Work In Progress

I'm not sure why CI is failing to clone. Have I specified the submodule incorrectly?

In any case, following the discussion today on IRC, I think it would really be nice to have GHC running these tests as part of its testsuite (and CI). That way, when someone wants to make a GHC-side change for which there is a corresponding Haddock tweak, there is a much smaller chance they'll inadvertently break Haddock's functionality. The only downside I see is that we'll need to add xml as a testsuite package...

/cc @RyanGlScott since you seemed particularly interested in this.

Adding a new submodule for the sake of running some tests seems quite heavyweight to me. Why do we not treat xml like the other test dependency library and only run the tests if it's installed and then modify CI to install it?

@mpickering, it's not clear to me what the policy is with respect to checking in submodules for libraries that are only required for running tests. For example, we check in the parallel submodule, despite it only being a testsuite dependency. On the other hand, we don't check in other testsuite dependencies like QuickCheck.

@harpocrates, as for the particular reason why CI is failing, I believe it thinks that xml's upstream lives on GitLab, which isn't the case (at least, not yet). I suppose the most direct way to work around this would be to add to the list of exceptional upstream locations here, alonside Haddock's GitHub upstream.

added 1 commit

• 5090a119 - Maybe fix the submodule?

Compare with previous version

Why do we not treat xml like the other test dependency library and only run the tests if it's installed and then modify CI to install it?

@mpickering That's exactly what I thought I'd done. Note that Hadrian handles this differently than make: it proactively tries to install test dependencies (see the treatment of parallel, for instance). What part of xml's handling looks suspicious to you?

@RyanGlScott Trying that now. I don't fully understand the .gitmodules url = ../packages/xml.git syntax and the packages file... Perhaps I've borked more than just CI. :)

@harpocrates I was thinking that there are some tests which use reqlib which only run when those libraries are installed. (Grepping for reqlib in the testsuite will show you which libraries I mean).

Right, reqlibs are what I was referring to in !291 (comment 5233). Note that parallel is a reqlib and has a submodule, while another reqlib, QuickCheck, does not have a submodule. It's unclear to me why.

Are you sure that's the reason parallel is a submodule? There are so few tests which reqlib it unless I am missing some. Do you know the commit the parallel submodule was added in? Has it always been there?

It seems the correct resolution here would be to remove the parallel submodule if it's only used for the testsuite rather than add the xml module.

It would be appropriate to pin the version of parallel and xml still I think but only to released versions which can be installed with cabal normally. Such a pinning could be achieved with a cabal.config file?

I'm not sure why parallel is a submodule. The first mention of it in the packages file is 2936ede2 and in the .gitmodules file is db19c665. There really does seem to be something special about parallel though - it's also the only package current listed in EXTRA_PACKAGES under ghc.mk.

Via some grepping, here are all the reqlibs in the testsuite:

QuickCheck
async
containers
ghc-prim
haskell98
integer-gmp
parallel
parsec
primitive
process
random
regex-compat
stm
syb
transformers
unix
utf8-string
vector
xml

It indeed looks like parallel is the exception rather than the rule.

What's the process for installing the libraries though? I don't see anything else in CI scripts for installing QuickCheck, for instance. Does that mean those tests aren't being run at all at the moment? I'm a bit concerned because the tests I'm proposing to add really are important. They would've caught at least half a dozen bugs in the last couple of months. It's a bit foolish to add tests if they'll never get run anyways.

Perhaps worth pointing out: QuickCheck might not build on ghc-head, but xml will almost certainly continue to compile on future GHC's. It really is quite a simple library with no real dependencies.

If there's no other way, I'll see if I can remove the xml dependency altogether (or at least do so conditionally) from html-test, latex-test, hoogle-test, and hypsrc-test.

@harpocrates This problem is tracked by #16264 (closed). I believe the solution to that ticket is to teach hadrian how to install packages from hackage and ask it to install specific versions before running the testsuite. Perhaps using head.hackage it will be possible to keep all these libraries working as GHC changes between versions.

marked as a Work In Progress

Removing xml as a dependency turned out to be surprisingly simple (https://github.com/haskell/haddock/pull/1027) so I guess backing out of the xml submodule changes is going to be the way forward here.