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

Requesting code coverage for a specific path misses lines #44593

Closed
IanButterworth opened this issue Mar 12, 2022 · 8 comments · Fixed by #44625
Closed

Requesting code coverage for a specific path misses lines #44593

IanButterworth opened this issue Mar 12, 2022 · 8 comments · Fixed by #44625
Labels
bug Indicates an unexpected problem or unintended behavior
Milestone
1.8

Comments

@IanButterworth
Copy link
Member

The option to track code coverage for files in a given path (#44359) appears to miss a significant number of lines compared to --code-coverage=user.

As seen in this Octavian CI run on julia nightly
JuliaLinearAlgebra/Octavian.jl#143

Screen Shot 2022-03-12 at 6 31 44 PM

Screen Shot 2022-03-12 at 6 36 37 PM
@IanButterworth IanButterworth added the bug Indicates an unexpected problem or unintended behavior label Mar 12, 2022
@IanButterworth
Copy link
Member Author

IanButterworth commented Mar 13, 2022 •
edited
Loading

I'm failing to find anywhere that the code introduced in #44359 differs for user vs. path mode in a way that would cause this bug.

Things I can confirm from studying the coverage tests, where path mode does miss some lines:

  1. The missing lines are not missing because they're being skipped in coverageVisitLine. I don't see any skipped there

    julia/src/codegen.cpp

    Line 2048 in 98b4b06

    if (filename == "" || filename == "none" || filename == "no file" || filename == "<missing>" || line < 0)

  2. The 3 missing lines aren't being skipped by any of the do_coverage checks

It seems like the skipping/missing is happening earlier

Another datapoint. All of the missing lines went through this branch in user mode

julia/src/codegen.cpp

Line 7159 in 98b4b06

coverageVisitLine(ctx, info.file, info.line);

Another: the first nlocs value is 3 less here in path mode

julia/src/codegen.cpp

Line 6989 in 98b4b06

size_t nlocs = jl_array_len(src->linetable);

and the same values further up via jl_array_len(src->linetable)

julia/src/codegen.cpp

Line 6344 in 98b4b06

else if (jl_array_len(src->linetable) > 0) {

user path
23 20
1 1
1 1
2 2
1 1
35 35
1 1
2 2
73 73
21 21
7 7
12 12

@IanButterworth
Copy link
Member Author

@c42f I wondered whether you have any advice here given it looks like you've fixed similar issues before?

My debugging is based on https://github.com/IanButterworth/julia/tree/ib/cov_debug
and running Base.runtests("cmdlineargs") which gives the following (annotated)

julia> Base.runtests("cmdlineargs")
Test    (Worker) | Time (s) | GC (s) | GC % | Alloc (MB) | RSS (MB)
cmdlineargs  (1) |        started at 2022-03-13T14:04:25.446
[ Info: ================ user ================
## other file visits omitted
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:11 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:14 0 0
## other file visits omitted
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 0 0
## other file visits omitted
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:11 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:18 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:1234 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:20 0 0
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0 # missing below
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 0 0
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 0 0 # missing below
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:20 0 0 # missing below
[ Info: ================ dir ================
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:11 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:14 1 1
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:3 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:4 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:5 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:7 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:8 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:9 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:12 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:10 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:18 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:1234 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:19 1 1
VISITED3 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:20 1 1
VISITED1 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:17 1 1
VISITED2 /Users/ian/Documents/GitHub/julia/test/testhelpers/coverage_file.jl:22 1 1
cmdlineargs  (1) |         failed at 2022-03-13T14:04:27.582

@IanButterworth
Copy link
Member Author

IanButterworth commented Mar 13, 2022 •
edited
Loading

My current theory is that #44359 didn't introduce the bug, but that the bug existed for user etc. given that the coverage tests have been against "bad" files, and the good references are @test_broken's

julia/test/cmdlineargs.jl

Line 294 in 98b4b06

expected = replace(read(joinpath(helperdir, "coverage_file.info.bad"), String),

Perhaps it's something like an indexing issue, and because there's fewer lines visited here, more of the lines of interest are missed.

@IanButterworth IanButterworth mentioned this issue Mar 13, 2022
Merged
@c42f
Copy link
Member

c42f commented Mar 14, 2022

@c42f I wondered whether you have any advice here given it looks like you've fixed similar issues before?

Hmm, I can't remember facing this. Do you have a particular prior issue in mind?

@DilumAluthge DilumAluthge added this to the 1.8 milestone Mar 14, 2022
@IanButterworth
Copy link
Member Author

@c42f I thought that #35138 was relevant, but perhaps not

@IanButterworth
Copy link
Member Author

AFAICT none of the code in codegen.cpp modifies src.linetable yet selecting different codecov options is causing different linetable sizes. See the last section in #44593 (comment)

I can't find anywhere outside of codegen.cpp where the jl_options.code_coverage influences anything, so I'm stuck.

@vtjnash does anything jump out to you here?

@vtjnash
Copy link
Member

vtjnash commented Mar 15, 2022

inference calls coverage_enabled(m::Module) to decide if it should emit coverage information for a statement

@IanButterworth IanButterworth mentioned this issue Mar 15, 2022
Merged
@IanButterworth
Copy link
Member Author

Ah! yes, that's it #44625
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Milestone
1.8
Development

Successfully merging a pull request may close this issue.

Fix code coverage in specific path mode IanButterworth/julia
4 participants
@vtjnash @c42f @IanButterworth @DilumAluthge