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

Turn of inlining when computing coverage (WIP) #866

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/API.jl
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ test(pkg::Union{String, PackageSpec}; kwargs...) = test([pkg]; kwargs...)
test(pkgs::Vector{String}; kwargs...) = test([PackageSpec(pkg) for pkg in pkgs]; kwargs...)
test(pkgs::Vector{PackageSpec}; kwargs...) = test(Context(), pkgs; kwargs...)

function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false, kwargs...)
function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false, inline=nothing, kwargs...)
pkgs = deepcopy(pkgs) # deepcopy for avoid mutating PackageSpec members
Context!(ctx; kwargs...)
ctx.preview && preview_info()
Expand All @@ -270,7 +270,11 @@ function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false, kwargs...
if !ctx.preview && (Operations.any_package_not_installed(ctx) || !isfile(ctx.env.manifest_file))
Pkg.instantiate(ctx)
end
Operations.test(ctx, pkgs; coverage=coverage)
if inline == nothing
# by default, allow inline for normal tests, and forbid it for coverage tests
inline = !coverage
end
Operations.test(ctx, pkgs; coverage=coverage, inline=inline)
return
end

Expand Down
3 changes: 2 additions & 1 deletion src/Operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,7 @@ function free(ctx::Context, pkgs::Vector{PackageSpec})
need_to_resolve && build_versions(ctx, new)
end

function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false)
function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false, inline=true)
# See if we can find the test files for all packages
missing_runtests = String[]
testfiles = String[]
Expand Down Expand Up @@ -1333,6 +1333,7 @@ function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false)
--color=$(Base.have_color ? "yes" : "no")
--compiled-modules=$(Bool(Base.JLOptions().use_compiled_modules) ? "yes" : "no")
--check-bounds=yes
--inline=$(inline ? "yes" : "no")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base this on Base.JLOptions().can_inline, similar to other options?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about doing that, too. One drawback of that would be that all packages affected by the problem at hand would have to be updated to explicitly add --inline=no to their test script(s), and of course also the default Julia scripts for Travis CI and AppVeyor would need to be adapted. I.e. far more people would have to change far more code.

That said, of course I can change it to do that shrug

--startup-file=$(Base.JLOptions().startupfile == 1 ? "yes" : "no")
--track-allocation=$(("none", "user", "all")[Base.JLOptions().malloc_log + 1])
--eval $code
Expand Down
4 changes: 4 additions & 0 deletions src/Pkg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ passing `coverage=true`. The default behavior is not to run coverage.

The tests are executed in a new process with `check-bounds=yes` and by default `startup-file=no`.
If using the startup file (`~/.julia/config/startup.jl`) is desired, start julia with `--startup-file=yes`.

If computing coverage has been requested, by default the new process is given
the option `--inline=no` to improve coverage computations. If that is not
desired, it can be avoided by passing `inline=true` to `test`.
"""
const test = API.test

Expand Down