Fix and test Pkg.test(pkg, coverage=true)

[timholy]

It turns out that in #9658 I broke Pkg.test(pkg, coverage=true), because the Cmd interpolation quoted a string with a space in it: I was generating julia --check-bounds=yes '--code-coverage=user --inline=no' ... rather than the unquoted form. Responsible for JuliaCI/travis-build#1 (comment). This should fix that.

Beyond the one-line fix, the much bigger part of this is adding a test.

EDIT: this part deleted due to feedback below
Finally, the last commit contains what I expect to be the most controversial point of this PR: I added "pkg" to the list of our default tests. Currently the problem is that this generates a LOT of console output, and one of the tests (PackageWithFailingTests) gives the appearance, to a casual/hasty observer, of some problem. We could presumably turn it off by redirecting the IO to /dev/null, at least on a Unix platform. I don't know about other platforms, though.

Another potential way to turn it off is with redirect_std{out,err}() (we'd have to cook up a way to do this across Cmd-launched processes), but if we go that way I'm worried about #9501. But maybe we should merge this as-is anyway. Then, the following sequence of events will occur:

• Multiple people will become annoyed by the amount of output generated during make testall
• One of those people will have the skills to fix Occasional "broken pipe" errors on Travis/AppVeyor during reflection test #9501, and will eventually get around to it. (That's a nasty bug, so we need to fix it anyway. This just raises the stakes.)
• Then we can turn off the output of test/pkg.jl using redirect_std{out,err}().

Thoughts?

[tkelman]

Finally, the last commit contains what I expect to be the most controversial point of this PR: I added "pkg" to the list of our default tests. Currently the problem is that this generates a LOT of console output, and one of the tests (PackageWithFailingTests) gives the appearance, to a casual/hasty observer, of some problem. We could presumably turn it off by redirecting the IO to /dev/null, at least on a Unix platform. I don't know about other platforms, though.

I don't think we can do this. That test relies on network access, which distribution buildbots don't have while building and running tests.

Note that the pkg test does run on Travis, where network access isn't an issue. I would run it on AppVeyor too but that introduces some Windows build-related complications (so this might fail there) involving win-extras. I think running things on Travis is sufficient here? If you're making local changes that influence pkg, you should make a point to locally run make test-pkg.

[tkelman]

Also given #8911 and #9654 (are those the only issues about coverage on windows, or are there others that I can't find?) this test may need to be @unix_only with a FIXME.

[timholy]

Excellent points, thanks the for the review, @tkelman.

I suppose an alternative is to add this kind of logic to runtests.jl:

web_required_for = ["pkg"]
web_on = true
try
    getaddrinfo("github.com")
catch
    warn("Web unavailable: Skipping tests [" * join(web_required_for, ", ") * "]")
    web_on = false
end

much like our current net_on-required tests. But I agree that running it on Travis should also be sufficient. Any thoughts on which is preferable?

[timholy]

Looks like pkg is being run on AppVeyor. I've just pushed your suggested @unix_only fix for that.

[tkelman]

Something like web_required_for might be a good idea long-term (not sure how different that would be from the current net_required_for though?). We should eventually figure out how to run the pkg test in the standard set. But I don't think we can do so at this time - we'll see whether it works without the coverage part on AppVeyor, but it will fail for all Windows source builds due to the aforementioned win-extras build complications. That would be inconvenient for myself and the other few people who do Windows source builds. We could also make this @unix_only there to help with that problem.

I do hope to resolve these Windows build issues that prevent source builds from having a fully-functional package manager, once the libgit2 transition starts making real progress towards migrating away from command-line git.

[tkelman]

cc @nalimilan on the web_required_for idea, I think he currently has to comment out some set of the tests on Fedora buildbots?

[timholy]

OK, I canceled the builds and pushed a new one that drops the part about adding to our list in runtests.jl.