reflection test failure when run with include_from_node1

[kshyatt]

When I run
julia --code-coverage=all

import(CoverageBase)
using Base.Test
CoverageBase.runtests(CoverageBase.testnames())

specifically with inlining, the tests immediately fail with:

reflection
The following 'Returned code...' warnings indicate normal behavior:
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
ERROR: LoadError: test failed: ("/Users/kshyatt/Projects/julia/test/reflection.jl" == "reflection.jl")
 in expression: (functionloc(foo7648,(Any,)))[1] == "reflection.jl"
 in error at error.jl:19
 in default_handler at test.jl:27
 in do_test at test.jl:50
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in anonymous at /Users/kshyatt/.julia/v0.4/CoverageBase/src/CoverageBase.jl:38
 in cd at file.jl:20
 in runtests at /Users/kshyatt/.julia/v0.4/CoverageBase/src/CoverageBase.jl:35
while loading /Users/kshyatt/Projects/julia/test/reflection.jl, in expression starting on line 150

which does not occur when I run the tests starting Julia with --inline-no.

[kshyatt]

So it seems the problem is that functionloc is finding the full path to reflection.jl, instead of a relative path. Should this test be changed somehow (maybe to a contains or "ends with" or something?)?

[tkelman]

The question is, is running with --code-coverage=all and/or importing CoverageBase expected to change the result of functionloc to an absolute path? The test is written expecting that it isn't, but if the changed result is expected then we could just work around it in the test.

[kshyatt]

I get the same behavior starting Julia with or without --code-coverage=all. I think the change was introduced in dd60925.

In particular this commit happened after Coveralls broke for a bunch of reasons, maybe that's why no one noticed it?

[kshyatt]

I just checked, if we change the test to use endswith all the tests pass, both make testall and using CoverageBase with --code-coverage-all.

Edit: tagging @JeffBezanson since it's his commit and he can probably explain what it's doing far better than I can!

[tkelman]

That commit was over a year ago and looks like it has very little to do with functionloc, are you sure that's what you meant?

[kshyatt]

Wow, I forgot it's not 2014! I got that from running a blame on the function. Maybe something in the test itself/Coveralls has changed? Sorry @JeffBezanson!

[simonster]

The test harness uses Core.include instead of ordinary include, which appears to make a difference:

julia> using Base.Test; include("reflection.jl")
The following 'Returned code...' warnings indicate normal behavior:
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
ERROR: LoadError: test failed: ("/usr/local/julia/test/reflection.jl" == "reflection.jl")
 in expression: (functionloc(foo7648,(Any,)))[1] == "reflection.jl"
 in error at error.jl:19
 in default_handler at test.jl:27
 in do_test at test.jl:50
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
while loading /usr/local/julia/test/reflection.jl, in expression starting on line 150

julia> using Base.Test; Core.include("reflection.jl")
The following 'Returned code...' warnings indicate normal behavior:
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.
Warning: Returned code may not match what actually runs.

julia>

[tkelman]

So include vs Core.include give rise to different behavior of functionloc, possibly by way of find_source_file? Is that intended?

[simonster]

Apparently Base.include is actually include_from_node1, which loads the file by its absolute path as opposed to the given relative path, so I suppose this is expected.

I suppose we're using Core.include in the harness to avoid having to pass the test files from node 1 to the workers when we run the tests in parallel, but it seems like a better solution might be to make workers always load files themselves if they're running on the same system as node 1.

It also looks like include_from_node1 is potentially vulnerable to a race condition when Julia is run in parallel, if another task gets run while it's sleeping and starts loading a file.

[kshyatt]

Sorry, I'm having a bear of a time finding where in CoverageBase/the buildbot Core.include is used instead of regular include. Could someone point me to it?

[tkelman]

Other way around. The runtests function that gets used for make test uses Core.include here

julia/test/testdefs.jl

Line 5 in 62b9817

tt = @elapsed Core.include("$name.jl")

, whereas CoverageBase is apparently using normal include / include_from_node1

[kshyatt]

Well, that explains it! I went into my copy of CoverageBase and changed include to Core.include and the reflection.jl problem is fixed, and other tests are running successfully. I checked launching with
julia -p 6 --code-coverage=all --inline=no
julia -p 6 --code-coverage=all
julia --code-coverage=all --inline=no
julia --code-coverage=all
Should someone submit a PR to CoverageBase about this, or wait until @simonster's issues get resolved?

[tkelman]

Should someone submit a PR to CoverageBase about this

That's probably a quick way of fixing this, but it's still a little iffy that using Base.Test; include("reflection.jl") doesn't work correctly. (edited, got it backwards at first)

[kshyatt]

Yeah, I think it makes sense for the "canonical" CoverageBase to not do weird hacky things like this. Since I want Coveralls so badly I guess I can just fork it and do the gross thing myself until the larger include issues get fixed.

[tkelman]

It's certainly worth seeing what other failures you hit in local coverage runs after resolving this one, even if in a temporary manner.

[tkelman]

the git blame was pretty misleading since the commit date on e75f2ce was nearly a year ago, but #7322 was only merged 10 days ago. Looks like Simon's got a fix in #11095.

[ihnorton]

Not sure if related, but there was a question about odd reload behavior on the list recently:
https://groups.google.com/forum/#!searchin/julia-users/reload%7Csort:date/julia-users/-_-d20qqR2w/xmhMIvLSs9cJ