[coverage] Simplify cobertura source paths for reporting.

Review Request #2918 — Created Oct. 1, 2015 and submitted

jtrobec
pants
jtrobec/coverage/cobertura-src-paths
2300
a405d3e...
pants-reviews
benjyw, stuhood

The existing approach to supplying cobertura reporting with access to coverage target source is somewhat complicated, largely with the intention of fooling cobertura into finding scala classes. Cobertura is not really designed to cover scala in the first place, so this seems fairly pointless. Removing that in favor of a much simpler approach that just passes the traget_base of relevant targets.

Manually tested on a couple of targets, and added validation to an existing integration test that looks for the annotated source file html and verifies it contains some src.

ST
  1. Can you clarify the effects of this change? Will scala go from having partial coverage to no coverage?

    1. It definitely will not go to "no" coverage, as I can get coverage reports for the example projects.

      The main issue, as I understand it from the older comments in the code, was that cobertura had some encoded assumptions w.r.t. where the source files should be for a given class. This follows java naming conventions, so it is apparently possible that if someone puts a scala file in an unusual location, it won't be found when cobertura generates the report. That means that annotated source showing lines covered would not be available for that class -- coverage numbers would still be available for the class since those are based on instrumented binaries.

      Against the hello/welcome example projects, I did get annotated scala with this approach. Then the other issue is just that line/branch coverage isn't necessarily sufficient for scala because of how compact a series of statements is...scoverage should introduce statement coverage which should be better for scala.

  2. 
      
ST
  1. Ship It!
  2. 
      
JT
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 8671207c2df7e4812c1681c5df68c27f6d935082

Loading...