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

--coverage flag for stack test #222

Closed
mboes opened this issue Jun 9, 2015 · 19 comments
Closed

--coverage flag for stack test #222

mboes opened this issue Jun 9, 2015 · 19 comments
Assignees
Milestone

Comments

@mboes
Copy link
Contributor

mboes commented Jun 9, 2015

Code coverage is a useful feature of GHC - it is particularly useful to figure out how much of the project's code is being exercised by tests. GHC ships with the hpc command, which can generate summary reports about source coverage and boolean-control coverage, as well as annotated versions of the source code with highlights where code was not covered.

The goal is to optionally obtain a coverage report every time the user says stack test, e.g. by adding a --coverage flag to that command.

The output of the hpc command is driven by *.tix files, generated by the test suite as a byproduct of execution when it is compiled using the -fhpc flag. So we have two choices:

  • reuse the hpc command-line line tool to generate reports that we store in some known location in the project directory.
  • generate our own reports, using the *.tix file. In particular for source code annotations, it may be useful to have that integrated with fpview.
@snoyberg snoyberg added this to the First stable release (0.1.0.0?) milestone Jun 9, 2015
@snoyberg
Copy link
Contributor

Assigning to @mgsloan.

@nh2
Copy link
Collaborator

nh2 commented Jun 15, 2015

Some problems I encountered in general with -fhpc:

Some problems that I believe I encountered and that might be useful to test whether Stack does right once this ticket is implemented:

@snoyberg
Copy link
Contributor

On the last one, I believe the requirement is "delete .mix and .tix files before running", is that correct?

I'm not sure if there's anything we can do about the other two at the stack level, unless I'm reading those incorrectly.

@snoyberg snoyberg assigned chrisdone and unassigned mgsloan Jun 15, 2015
@nh2
Copy link
Collaborator

nh2 commented Jun 16, 2015

I'm not sure if there's anything we can do about the other two at the stack level, unless I'm reading those incorrectly.

Regarding the GHC bug, we might choose to work around them, for example to warn the user that coverage only works with -O0 with the GHC version that the project is using.

Regarding the bad error messages: The adversarial case appears (seems to appear, I haven't fully understood it yet) when you're running (generates the .tix) a slightly different version of a program/linked library than when you were compiling (generates the .mix). Since we may have control over compiling/running, we might be able to ensure that at the those files are up-to-date in relation to each other.

@snoyberg snoyberg modified the milestones: 0.2.0.0, 0.1.0.0 Jun 22, 2015
@chrisdone
Copy link
Member

FYI: Encountered that message while implementing this. Removing the file before running the suite does the trick.

chrisdone added a commit that referenced this issue Jun 23, 2015
@chrisdone
Copy link
Member

Okay, implemented. The output is like this:

41 examples, 0 failures
Generating HPC HTML ...
 23% expressions used (3736/16040)
  7% boolean coverage (12/159)
       8% guards (7/82), 15 always True, 6 always False, 54 unevaluated
       8% 'if' conditions (5/61), 6 always True, 4 always False, 46 unevaluated
       0% qualifiers (0/16), 16 unevaluated
 15% alternatives used (122/776)
 36% local declarations used (197/541)
 19% top-level declarations used (248/1249)
The HTML report is available at /home/chris/Work/stack/.stack-work/dist/x86_64-linux/Cabal-1.18.1.5/hpc/hpc_index.html

It seems that one has to do a clean build from scratch otherwise the hpc program will complain. It turns out that doing a build from scratch with profiling enabled is rather problematic, leading to weird linking errors. I'm trying to figure out why.

@chrisdone
Copy link
Member

Okay, so I tracked it down, here's the problem:

Running stack build --library-profiling --executable-profiling twice in a row results in:

src/main/Main.hs:74:29:
    cannot find normal object file ‘.stack-work/dist/x86_64-linux/Cabal-1.18.1.5/build/stack/stack-tmp/Paths_stack.dyn_o’
    while linking an interpreted expression

We're not sure why. Suspected Cabal library issue. The icky workaround, to re-generate an HPC report, is to run:

stack clean; stack build; stack test --coverage

@snoyberg
Copy link
Contributor

Possible solution would be using a newer version of the Cabal library ala #174. We can touch base tomorrow on how we could test this out.

@snoyberg
Copy link
Contributor

snoyberg commented Jul 2, 2015

Passing to @mgsloan to finish implementation.

@snoyberg snoyberg assigned mgsloan and unassigned chrisdone Jul 2, 2015
@mgsloan
Copy link
Contributor

mgsloan commented Jul 9, 2015

While this could be hacked around at the cabal level, this is a ghc issue - incremental recompilation doesn't notice when a .mix file is out of date or missing. So, if you do a normal build, and then ask for -fhpc, won't emit new .mix files, since the .o files are uptodate.

One solution would be to use -fforce-recomp, but this isn't much better than doing a clean + test.

@mgsloan
Copy link
Contributor

mgsloan commented Jul 9, 2015

I also ran into an issue when getting coverage information for a multi-package project. Since all the packages are build with coverage, we get coverage data for all of them in the .tix files. This means that when generating the report, we need to provide hpc with all of the packages in the project.

We may want to make it an option to only record coverage info for some subset of the packages, but this default is quite nice. It means that you get coverage information across your packages, rather than for a single one.

Fix here: ec043d1

@mgsloan
Copy link
Contributor

mgsloan commented Jul 9, 2015

Cabal files with multiple test suites are also problematic. I have an initial fix for calling hpc correctly in these cases: 66deefa

However, that doesn't make it work for a test project: https://github.com/mgsloan/multi-test-suite - here's the output:

multi-test-suite-0.1.0.0: test (suite: multi-test-suite-test)
Test suite not yet implemented
multi-test-suite-0.1.0.0: test (suite: multi-test-suite-test-2)
Test suite not yet implemented
Generating HPC HTML ...
100% expressions used (2/2)
100% boolean coverage (0/0)
     100% guards (0/0)
     100% 'if' conditions (0/0)
     100% qualifiers (0/0)
100% alternatives used (0/0)
100% local declarations used (0/0)
100% top-level declarations used (1/1)
The HTML report is available at /home/mgsloan/fpco/multi-test-suite/.stack-work/dist/x86_64-linux/Cabal-1.18.1.5/hpc/multi-test-suite-test-2/hpc_index.html
Generating HPC HTML ...
Running /home/mgsloan/.ghc/bin/hpc markup --destdir /home/mgsloan/fpco/multi-test-suite/.stack-work/dist/x86_64-linux/Cabal-1.18.1.5/hpc/multi-test-suite-test/ multi-test-suite-test.tix --srcdir /home/mgsloan/fpco/multi-test-suite/ --hpcdir .stack-work/dist/x86_64-linux/Cabal-1.18.1.5/hpc/.hpc/ --reset-hpcdirs exited with ExitFailure 1
hpc: can not find Main in ["/home/mgsloan/fpco/multi-test-suite//.stack-work/dist/x86_64-linux/Cabal-1.18.1.5/hpc/.hpc/"]

While it says it can't find Main, the file is certainly there. Opening up the Main.mix file makes it clear that it's the one intended for the other test-suite, multi-test-suite-test-2. So, it looks like the .mix files get clobbered in the presence of multiple test-suites... That's going to be a bit of a pain to resolve.

@mgsloan
Copy link
Contributor

mgsloan commented Jul 9, 2015

That's a bit of a comment deluge, so, here's a summary of what's left:

  • Fix the ghc recompilation issue. A nice option here might be to only force recompilation if the prior flags didn't include -fhpc.
  • Support multiple test suites, by resolving the issue where the test-suite's .mix files clobber eachother. EDIT: resolved by emitting a warning message letting the user know that they will only get an HPC report for the last test in a multi test-suite cabal file. I'd say this is a bug in how GHC does HPC / the hpc report program. We could hack around it, but it'd be ugly.

@mgsloan
Copy link
Contributor

mgsloan commented Jul 13, 2015

So, the remaining issue is to solve the problem @chrisdone encountered, where sometimes .mix files go missing. The reason for this is described above.

One sensible approach would be to touch source files whenever their .mix file is out of date. This ought to work well. However, this requires us to figure out the mapping from module names to source files. It looks like cabal already has utilities for doing this: https://github.com/haskell/cabal/blob/master/Cabal/Distribution/Simple/Haddock.hs#L715

I noted that there isn't one of these get*SourceFiles functions for tests or benchmarks. Why is this? Because they don't list their modules. This means the approach based on info from the cabal file is sunk.

What's an alternative? We can look for all the .p_o / .dyn_o files in the dist build directory, and figure out the corresponding paths in the hpc directory. Then, if the .mix file is older than the .p_o file, delete the .p_o file to force recompilation. This will require some experimentation - if the .mix is always written before the output, this will work nicely. Otherwise, we may need to fudge it and add some small time interval where the .mix is allowed to be older.

As far as I can tell, this is the only tenable option to get to a nice user experience. Other options:

A) Have users cabal clean before doing test coverage. Maybe suggest this when the hpc program fails with an error.

B) Have cabal test --coverage build everything with -fforce-recomp, forcing everything to be recompiled.

@mgsloan
Copy link
Contributor

mgsloan commented Jul 16, 2015

Happily, it seems that using --ghc-options -hpcdir ... fixed the incremental recompilation issues. I'm not 100% confident of that, but I haven't been able to cause any issues by editing files and intermixing stack build with stack test --coverage.

I've documented the main issue I know of here

@chrisdone Can you please confirm that the current state of test --coverage functions well and doesn't exhibit the behavior described here?

@chrisdone
Copy link
Member

Can do.

@chrisdone
Copy link
Member

Confirmed that I can re-run stack test --coverage reliably. The same problem persists if I stack clean and then stack test --coverage. But stack clean; stack build; stack test --coverage works.

Cleaning is rarer, but this bug may bite some people. At any rate, one can at least re-run stack test --coverage and iterate like that.

@mgsloan
Copy link
Contributor

mgsloan commented Jul 16, 2015

@chrisdone Thanks for checking! Looks like this is a bug with the profiling support (see #608), rather than with test coverage. I've made it so that test coverage doesn't use profiling flags, which I think fixes this issue. Confirmed?

I can't quite do your repro because I run into another funky issue: #614

@mgsloan
Copy link
Contributor

mgsloan commented Aug 4, 2015

I think if there are any more issues with --coverage, additional issues can be opened - closing this one.

@mgsloan mgsloan closed this as completed Aug 4, 2015
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

5 participants