Add the force_latest_compat keyword argument to Pkg.test

[DilumAluthge]

This pull request adds the force_latest_compat keyword argument to the Pkg.test function.

When force_latest_compat is true, then, for each [compat] entry in the active project, only the latest compatible version will be allowed.

The default value is force_latest_compat = false.

Motivation

Suppose that you have a bot (e.g. CompatHelper or Dependabot) that opens pull requests to widen your [compat] entries whenever one of your dependencies makes a new breaking release.

And suppose that originally your package's Project.toml file looks like this:

[compat]
SomePackage = "1"

The bot opens a PR to change your package's Project.toml file to this:

[compat]
SomePackage = "1, 2"

Now, your CI runs your test suite with this new Project.toml file. The problem is that, during the test suite, Pkg.test might choose to run your package's tests with version 1 of SomePackage.jl. Therefore, even if your CI tests pass, you don't know whether or not your package is compatible with version 2 of SomePackage.jl.

The solution is to modify your CI test script. Instead of using this as your test script (in Travis CI, AppVeyor CI, GitHub Actions CI, etc.):

Pkg.test(coverage=true)

You would have this as your CI test script instead:

Pkg.test(coverage=true, force_latest_compat=true)

This will guarantee that your tests will run with version 2 of SomePackage.jl.

[KristofferC]

If this is decided to be a good idea, I think an option to Pkg.test makes more sense instead of actual rewriting of the project file.

[DilumAluthge]

Sure, that would be fine. We could have an optional keyword argument force_latest_compat. Where in the Pkg.test code would I implement this?

I definitely think that we need this feature somewhere. Otherwise, you don't actually know if your CompatHelper/Dependabot PRs are passing CI.

[DilumAluthge]

@KristofferC Alright, it is now a keyword argument to Pkg.test. Example usage:

Pkg.test(coverage = true, force_latest_compat = true)

The default value is force_latest_compat = false.

[DilumAluthge]

Bump. @KristofferC take a look at the new implementation and let me know what you think.

[DilumAluthge]

Bump.

[christopher-dG]

Just to put what's been said in Slack somewhere permanent:

It might be a good idea to make forcing latest versions of dependencies the default Pkg.test behaviour in CI environments for PRs created by CompatHelper or Dependabot. For example, in GitHub Actions the environment variable GITHUB_REF will be refs/heads/compathelper/new_version/....

Personally, I don't see this getting much buy-in if it needs to be done manually.

[DilumAluthge]

Just to put what's been said in Slack somewhere permanent:

It might be a good idea to make forcing latest versions of dependencies the default Pkg.test behaviour in CI environments for PRs created by CompatHelper or Dependabot. For example, in GitHub Actions the environment variable GITHUB_REF will be refs/heads/compathelper/new_version/....

Personally, I don't see this getting much buy-in if it needs to be done manually.

I agree.

Do you think that should go in this PR, or a follow-up PR? I was thinking that we first get this PR merged, and then I'll make a follow-up PR to add the automatic CI detection.

The alternative approach would just to be to set force_latest_compat = true by default and call it a day. But I imagine that people will not like that.

[christopher-dG]

If we're going to make latest compat default all the time, it's basically the same as just not having the option to at all. Not a good idea IMO. It doesn't really matter to me in which PR stuff happens, just as long as it doesn't get delayed a whole minor Julia version. If it's easier to do it after this is merged, then sure.

[DilumAluthge]

How about this: I'll put the CI changes in a new PR targeting this branch. So Kristoffer and others can review the two PRs separately. But if they do want to review everything at once, they can merge that PR into this one.

I'll get the CI changes PR up later today.

[DilumAluthge]

Okay, the "set force_latest_compat=true on Dependabot/CompatHelper" pull request is up here: #2234

@KristofferC Do you want to review these two pull requests (#2176 and #2234) separately? Or do you want me to merge them into a single branch, and then you can review it as a single pull request?

[DilumAluthge]

Based on the discussion in #2234, I have closed #2234, and the "auto-detect Dependabot/CompatHelper" functionality has been moved into julia-actions/julia-runtest#20.

[quinnj]

Bump; this seems really useful for CompatHelper

[DilumAluthge]

Any additional thoughts or review comments on this PR? If not, I'm inclined to wait a little bit, and then merge this PR.

[KristofferC]

This function looks kinda pointless. We already have functions to read and write projects. What you want to do is just change the compat of an existing project. So the only thing that should be modified is

Pkg.jl/src/Types.jl

Line 210 in 8b0045d

compat::Dict{String,String} = Dict{String,String}()# TODO Dict{String, VersionSpec}

.

[DilumAluthge]

Sure, we can get rid of the force_latest_compat(filename::AbstractString) method. However, we would just end up moving this code (the code to load the project from the Project.toml file) from this location to line 1443 of src/Operations.jl.

If you look at lines 1437 through 1443 of src/Operations.jl: the temp project that is going to be used for the test sandbox is in the form of a Project.toml file that has been written to disk at the filename tmp_project. This temp project does not exist in memory yet; it is only a project file on disk. So we will need to read it in from the file.

[KristofferC]

Modify the sandbox_project_override.env.project.compat before it is written (at line 1437) and the correct compat should just be printed automatically. No need to write to disk, chmod, read it back in, modify compat, write it back out to disk, chmod again.

[DilumAluthge]

That works if sandbox_project_override !== nothing. But what if we are in the other branch, i.e. isfile(sandbox_project)?

[DilumAluthge]

See #2439 for an alternative to this PR.

[DilumAluthge]

Closing in favor of #2439