Pkg.jl: test resolves from wrong manifest

[fredrikekre]

I added Example to Literate/test/Project.toml (and no test/Manifest.toml file):

(v1) pkg> st
    Status `~/.julia/environments/v1/Project.toml`
  [7876af07] Example v0.5.0
  [98b081ad] Literate v1.0.4 [`~/dev/Literate`]

(v1) pkg> test Literate
[...]
 Resolving package versions...
  Updating `/tmp/tmpE0z0sa/Project.toml`
  [7876af07] + Example v0.5.1            <-- Should be v0.5.0 from my v1 environment.
  [8dfed614] + Test

[00vareladavid]

Why does the v1 env matter here? It should use whatever Example is in Literate/test/Manifest.toml, no?

[fredrikekre]

Right, I should add that I did not have a test/Manifest.toml file.

[00vareladavid]

Then it should resolve test/Project.toml, which will pull down the latest Example

[fredrikekre]

It makes more sense to test the package in the installed env. If I have [email protected] I want to use that, no? This is how things behaved pre-#989.

(This is what I was trying to explain on the last call.)

[fredrikekre]

So, IMO, it should behave like this if there is no test/Manifest.toml

1. Collect from $current_env/Manifest.toml
2. If still not fully satisfied, do a resolve.

[00vareladavid]

Ah, I see. That does seem reasonable. I don't really have a strong preference at this point.

[00vareladavid]

I'll try to move to that system, seems like it should work.

[00vareladavid]

I've thought about this some more. I'm worried reusing the parent dependency graph will be too brittle. We currently do not have much ability to gracefully degrade from the best case. In other words: we can try to reuse the dependency graph, and possibly we get lucky. If we don't get lucky, we basically just resolve from scratch. Its basically an "all or nothing" situation. I feel uncomfortable providing a feature that we don't have much control over. How strongly do you feel about supporting this behavior?

A possible alternative would be to match the compat section of specific nodes that you care about. I'm more comfortable with this because it would reuse mechanisms that are exposed outside of the test operation. This approach would of course be more manual, but (imo) also more transparent behavior.

[fredrikekre]

I think this is important. Without this pkg> test Package does not mean anything if the package testing can use whatever versions they want.

[00vareladavid]

It occurs to me that we are possibly thinking of different testing scenarios. I am focusing on testing a package as a dependency. It seems (correct me if I'm wrong) you are focusing on testing a package as an application.

When using a package as an application, Pkg will use the dependency graph in the manifest file. In this case, it makes perfect sense to preserve the dependency graph as much as possible. It would be cool if the resolver could preserve this property, but I think a "best effort" approach should be good enough for now.

When using a package as a dependency, Pkg will not preserve the whole dependency graph. In this case, I don't think it makes sense for test to resolve in a special manner. test should just reuse the mechanisms (such as compat) which are used during a regular resolve.

I think test can support both.

[fredrikekre]

It occurs to me that we are possibly thinking of different testing scenarios. I am focusing on testing a package as a dependency. It seems (correct me if I'm wrong) you are focusing on testing a package as an application.

I am thinking about

(v1) pkg> add Example; test Example

i.e. testing Example as a dependency to my (v1) project here. If we don't try to collect deps from (v1) testing becomes something like https://github.com/auchenberg/volkswagen

[fredrikekre]

This does not seem to have been fixed?

(v1) pkg> st
    Status `~/.julia/environments/v1/Project.toml`
  [7876af07] Example v0.5.0
  [98b081ad] Literate v1.0.4 [`~/dev/Literate`]

shell> cat dev/Literate/test/Project.toml
[deps]
Example = "7876af07-990d-54b4-ab0e-23690620f79a"
Literate = "98b081ad-f1c9-55d3-8b20-4c87d4299306"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

(v1) pkg> test Literate
   Testing Literate
 Resolving package versions...
  Updating `/tmp/tmptFuaNb/Project.toml`
  [7876af07] + Example v0.5.1
  Updating `/tmp/tmptFuaNb/Manifest.toml`
  [7876af07] + Example v0.5.1
[ Info: Running Sandbox
    Status `/tmp/tmptFuaNb/Project.toml`
  [7876af07] Example v0.5.1        <-- Should be v0.5.0 from my v1 environment.
  [98b081ad] Literate v1.0.4 [`~/dev/Literate`]
  [8dfed614] Test

[00vareladavid]

So #1033 is about reusing a subgraph of the parent dependency graph for testing. The idea is that you preserve the directed subgraph of the node you are testing, and add to that subgraph the test nodes. So if you run test Literate, then Literate's directed subgraph (i.e. its direct dependencies and recursive dependencies) is preserved. Since Example is not in that subgraph, its version is not preserved.

If v1 was a project and you ran Pkg.test(), then both Example and Literate's version is preserved, because they would both be in the subgraph of v1.

In other words, if you want to test the interaction between Example and Literate (which I assume is your intention here), then you have to test a node which links both.

[KristofferC]

So are we missing this part now? https://github.com/JuliaLang/Pkg.jl/blob/f3c1ce1e81093d0377892d1c99d89ee4474a24aa/src/backwards_compatible_isolation.jl#L488-L500

Basically, we want to look at the pkg we test, collect all its dependencies and test dependencies. Recursively collect all the dependencies of those. Whatever of those packages we have in the current manifest we put in the temporary manifest at the versions of the current manifest.

[00vareladavid]

The current implementation collects (preserves) the recursive dependencies of the package you are testing.

What it does not do is preserve the test dependencies which are not recursive dependencies of the package you are testing. It is not clear to me why this is useful behavior.

[00vareladavid]

In other words: Say, as in the above example, you want to test the interaction of Example and Literate. Is that not exactly the case where you would run Pkg.test() instead of Pkg.test("Literate")? The sandbox will preserve the versions of both dependencies in this case. Why would you single out Literate if the intention is to test both packages?

[fredrikekre]

The idea with the example is that I want to test how Literate behave in the environment where it is being used. What you write makes sense, to reuse the subgraph for Literate from v1, but when we realize we need Example we should first look for it in the current manifest (v1) and if it is not found there resolve a version of it.

[KristofferC]

IIRC, I put the behavior of recursive collecting dependencies of the package getting tested from the current manifest in after Chris Rackauckas complained that his dev packages was not getting used when testing. So from that interaction it seemed like people believe that what they have in their current manifest is what will be used when testing their packages.

[00vareladavid]

Perhaps I misunderstand what you are saying, but

recursive collecting dependencies of the package getting tested from the current manifest

is what the current sandbox is doing. Running test Literate will preserve Literate and its recursive dependencies.

[00vareladavid]

I truly believe that using Pkg.test to specify the root of the subgraph you wish to preserve/test and using test/Project.toml to specify "I want these packages available to poke at the subgraph" is a healthy separation.

[fredrikekre]

Packages can (and often do) interact without having a dependence on each other. Example: Lets say I have FooBase and Foo in my v1 environment. Foo depends on FooBase, and FooBase has Foo as a test dependency. Now, test FooBase should (obviously) use the Foo from v1, otherwise how do I verify that my v1 environment is working as expected? Even worse, from https://github.com/JuliaLang/Pkg.jl/issues/1030#issuecomment-466184050, if I am actively developing them both, and I want to verify that they still work, should I not be able to test FooBase?

[00vareladavid]

If you want to test the interaction between Foo and FooBase why not run test Foo? test Foo will preserve the versions of both Foo and FooBase.

[00vareladavid]

Packages can (and often do) interact without having a dependence on each other

I have not said anything which contradicts that. If A depends on both B and C, and you want to test how B and C interact, then you should test the environment where they interact i.e. test A. The current sandbox fully supports this.

Running test A will preserve both B and C. There is no requirement for B to depend on C or vice versa.

[fredrikekre]

How am I supposed to verify that test FooBase works as expected with my in-development version of Foo?

[00vareladavid]

You can test that FooBase works with your in-development version of Foo with test Foo.

[fredrikekre]

Yea, but that is not running the tests for FooBase.

[00vareladavid]

If you want to test FooBase at the existing version, run test FooBase.

[fredrikekre]

But

FooBase has Foo as a test dependency.

?
There might not even exist a released version that is compatible with my new FooBase.

[00vareladavid]

Yes, but I believe this example is a bit contrived. Why would you add tests for Foo in FooBase/test/runtests.jl? Don't they belong in Foo/test/runtests.jl? Both Foo and FooBase will be available at the preserved version in that case.

So if you want to test if Foo works with your development version of FooBase:

activate Foo
develop /path/to/FooBase
test

I think this is reasonable.

[KristofferC]

Running test Literate will preserve Literate and its recursive dependencies.

Including test dependencies. During testing time the test dependencies are added to the dependency graph of the package. Then the recursive dependencies are collected.

[fredrikekre]

Right, the dependencies when testing Literate should be Literate/Project.toml/[deps] + Literate/test/Project.toml/[deps].

[00vareladavid]

During testing time the test dependencies are added to the dependency graph of the package. Then the recursive dependencies are collected.

Why?

I understand the difference between the current and previous isolation mechanisms. It took significant effort to replace the previous isolation mechanism. Now both of you are asking to reintroduce the complexity that existed in the previous implementation without providing a compelling argument as to why it is superior.

The current "sandbox" isolation mechanism has been able to cleanly satisfy every intention that you have listed.

[00vareladavid]

resolved by #1093