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

Add Compiler flag to specify --code-coverage #2822

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

disberd
Copy link
Contributor

@disberd disberd commented Feb 21, 2024

This PR adds two compiler flags to specify how to track code coverage in the notebook session.

I created two distinct flags as the command line option --code-coverage can be provided 2 times:

  • One for specifying a file where to store the coverage data.
    • By default, if coverage is tracked, a separate coverage file per source file is generated. Specifying a file explicitly saves everything in a single file.
  • Another for specifying what to track, it can be none, user, all or @<path> for a specific path.

The --code-coverage=@<path> was added in 1.8.

I mention this in the docstring but I do not do validation of the content of the provided inputs to the flag as that is not done for other compiler flags.

Fixes #2818

Copy link
Contributor

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/disberd/Pluto.jl", rev="code_coverage_flag")
julia> using Pluto

@disberd
Copy link
Contributor Author

disberd commented Feb 21, 2024

I guess fails on julia nightly are not related to this PR

src/Configuration.jl Outdated Show resolved Hide resolved
@fonsp
Copy link
Owner

fonsp commented Feb 26, 2024

Thanks! Looks good, it's pretty weird API but that's because the --code-coverage API is weird 🙃

I'm not sure about the tests, I feel like this is more a test of Julia's internals, and less of Pluto's wrapper around it. It also launches a couple processes in sequence which makes the tests longer.

Maybe you could write a test that is closer to the way you intend to use it, and tests that it produces the result that you want? I tried this:

julia> options = Pluto.Configuration.from_flat_kwargs(; launch_browser=false, code_coverage_file="test.info", code_coverage_track="all");

julia> 🍭 = Pluto.ServerSession(; options);

julia> testfile = download(
           "https://raw.githubusercontent.com/fonsp/Pluto.jl/main/sample/Tower%20of%20Hanoi.jl",
       )
"/var/folders/v_/fhpj9jn151d4p9c2fdw2gv780000gn/T/jl_gBkxuXU1Sb"

julia> nb = Pluto.SessionActions.open(🍭, testfile; run_async=false);

julia> contents = read("test.info", String)
"SF:/Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-release-1-dot-10/usr/share/julia/stdlib/v1.10/Pkg/src/utils.jl\nDA:2,3\nDA:3,1\nDA:4,1\nDA:5,1\nDA:6,1\n"  22634 bytes  ",0\nDA:456,0\nDA:458,0\nDA:459,0\nDA:460,0\nDA:461,0\nDA:462,0\nDA:463,0\nDA:465,0\nDA:517,0\nDA:519,389\nDA:538,2\nDA:540,0\nDA:541,0\nDA:542,0\nDA:543,0\nDA:574,4\nDA:575,0\nDA:576,394\nDA:579,0\nDA:642,199\nDA:645,24\nDA:884,0\nDA:885,0\nDA:886,0\nDA:887,0\nDA:889,0\nLH:9\nLF:35\nend_of_record\n"

julia> fn = basename(testfile)
"jl_gBkxuXU1Sb"

julia> occursin(fn, contents)
false

julia> # weird

This actually looks like the notebook file is not included in the coverage...

@disberd
Copy link
Contributor Author

disberd commented Feb 27, 2024

@fonsp Yeah your comment makes sense, I changed the tests to only do a single test to verify that a coverage file is created for some package under development (I used Example.jl as example :D).

I also noticed in my tests that code coverage for code touched by a notebook is only generated after closing the workspace_session, which might be why in your test you couldn't see the notebook lines.

@disberd
Copy link
Contributor Author

disberd commented Feb 27, 2024

Not sure why this fails on macos 1.10 and I do not have a Mac to debug :(

@fonsp
Copy link
Owner

fonsp commented Aug 10, 2024

what to do with this pr? for me its a lot of work for a feature that i dont imagine using :o but it would be nice to have

@disberd
Copy link
Contributor Author

disberd commented Aug 10, 2024

If I get access to a Mac I can try fixing it, (1.11 test breaks do not seem to happen in the code touched by this), but doing that just relying on CI is indeed a lot of work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running Pluto with Malt.jl inside CI tests does not update code coverage
2 participants