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.

Sign up for GitHub

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

Jump to bottom

Compiler produces incorrect/incomplete coverage data #16634

Closed
rolandtritsch opened this issue Jan 8, 2023 · 3 comments
Closed

Compiler produces incorrect/incomplete coverage data #16634

rolandtritsch opened this issue Jan 8, 2023 · 3 comments
Labels
area:coverage Code coverage, see https://dotty.epfl.ch/docs/internals/coverage.html itype:bug stat:needs spec

Comments

@rolandtritsch
Copy link

See also - scoverage/sbt-scoverage-samples#28

Compiler version

3.2.2-RC2

Minimised code

See issue above.

To reproduce run sbt +clean +coverage +test +coverageReport on sbt-scoverage-samples.

Output

See issue above. Output for scala3 is incorrect.

incorrect

Expectation

Generated scala3 scoverage.coverage data should (roughly) equal the scala2 coverage data.

Next step(s)

  • Can you verify/confirm that this is not a known issue
  • (Optional; if needed) Make the test case smaller (@rolandtritsch)
@rolandtritsch rolandtritsch added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 8, 2023
@mbovel mbovel added area:coverage Code coverage, see https://dotty.epfl.ch/docs/internals/coverage.html and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 9, 2023
@Kordyjan Kordyjan added the stat:needs minimization Needs a self contained minimization label Jan 13, 2023
@Kordyjan Kordyjan added this to the Future versions milestone Jan 13, 2023
@Kordyjan Kordyjan added stat:needs spec and removed stat:needs minimization Needs a self contained minimization labels Jan 13, 2023
@Kordyjan Kordyjan removed this from the Future versions milestone Jan 13, 2023
@vijaykramesh
Copy link

Likely related to scoverage/sbt-scoverage#382 as well as scoverage/sbt-scoverage#458

@vijaykramesh
Copy link

vijaykramesh#1 demonstrates the issue pretty clearly by adding an additional coverage test

@vijaykramesh vijaykramesh mentioned this issue Apr 13, 2023
Closed
KacperFKorban referenced this issue in dotty-staging/dotty Aug 22, 2023
@KacperFKorban
Coverage: mark case bodies as branches; don't ignore branches with sy…
bcb2053 
…nthetic spans

Marking bodies of CaseDefs as branches seems like an uncontroversial
change, it was probably an oversight.

Not ignoring synthetic spans when creating coverage calls in branches
seems like a good trade off. There might be some auto-generated `else
()` interpreted as branches, but Scala introduces quite a lot of
synthetic trees that wrap non-synthetic trees (e.g. implicit classes).
Also, it looks like Scala 2 includes those compiler-generated `else`
branches in coverage. (Another possibility here would be to also check
if the span is zero extent, but that approach would be different to the
Scala 2 one)

partial fix for lampepfl#16634
@KacperFKorban KacperFKorban mentioned this issue Aug 22, 2023
Merged
sjrd added a commit that referenced this issue Aug 22, 2023
@sjrd
Coverage: mark case bodies as branches; don't ignore branches with sy…
4347d29 
…nthetic spans (#18437)

Marking bodies of CaseDefs as branches seems like an uncontroversial
change, it was probably an oversight.

Not ignoring synthetic spans when creating coverage calls in branches
seems like a good trade off. There might be some auto-generated `else
()` interpreted as branches, but Scala introduces quite a lot of
synthetic trees that wrap non-synthetic trees (e.g. implicit classes).
Also, it looks like Scala 2 includes those compiler-generated `else`
branches in coverage. (Another possibility here would be to also check
if the span is zero extent, but that approach would be different to the
Scala 2 one)

partial fix for #16634
@KacperFKorban
Copy link
Member

Closing this issue now. It has been addressed in:

Here is the coverage comparison using the current main:
image

Kordyjan referenced this issue Dec 11, 2023
@KacperFKorban @Kordyjan
Coverage: mark case bodies as branches; don't ignore branches with sy…
3661710 
…nthetic spans

Marking bodies of CaseDefs as branches seems like an uncontroversial
change, it was probably an oversight.

Not ignoring synthetic spans when creating coverage calls in branches
seems like a good trade off. There might be some auto-generated `else
()` interpreted as branches, but Scala introduces quite a lot of
synthetic trees that wrap non-synthetic trees (e.g. implicit classes).
Also, it looks like Scala 2 includes those compiler-generated `else`
branches in coverage. (Another possibility here would be to also check
if the span is zero extent, but that approach would be different to the
Scala 2 one)

partial fix for lampepfl#16634

[Cherry-picked bcb2053]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:coverage Code coverage, see https://dotty.epfl.ch/docs/internals/coverage.html itype:bug stat:needs spec
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@vijaykramesh @rolandtritsch @mbovel @Kordyjan @KacperFKorban