Copy preferences into the test environment - #121
davidanthoff wants to merge 8 commits into
Conversation
This code is based on what Pkg does, as identified here: #59 (comment) - Check if there are LocalPreferences.toml in test/ - If not, check in top-level package path - Copy whatever LocalPreferences.toml you find into the tmp env.
Move the preference lookup into `common.jl` as `sandbox_preferences`, shared by both `activate` forms, and apply it to the julia-1.8, 1.9, 1.11, 1.12 and 1.13 directories rather than just julia-1.9. Julia 1.7 and earlier are left alone: `Base.get_preferences` there only exposes preferences per package UUID, so there is no way to collect the whole merged set to copy across. Along the way: - `Base.get_preferences()` returns an empty `Dict` rather than `nothing` when there are no preferences, so the old `!== nothing` guard always fired and always wrote an empty preferences file. Guard on `!isempty` instead. - Drop the `source_path` local, which shadowed `Pkg.Operations.source_path`. - Qualify `copy!(Base.LOAD_PATH, ...)` in the restore path. - Answer the `# TODO: should we separately import TOML?` in a comment: no -- TestEnv supports Julia 1.0, which predates the `TOML` stdlib. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`TestEnv.activate(f, pkg)` delegates to `Pkg.Operations.sandbox`, which has taken a `preferences=` keyword since Julia 1.8 but was never passed one, so that form dropped preferences even with the set form fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `isdefined(@__MODULE__, :orig_load_path)` guards were never true -- those are locals inside the testset, not module globals -- so the `finally` block silently did nothing and both globals leaked into every later testset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three offline fixture packages under test/preferences/ (no registry deps, so they instantiate without network): preferences at the package root only, in `test/` shadowing the package root, and none at all. Both `activate` forms are covered, plus the negative case that no preferences means no preferences file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
+ Coverage 81.69% 83.36% +1.67%
==========================================
Files 34 34
Lines 945 1040 +95
==========================================
+ Hits 772 867 +95
Misses 173 173 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| tmp = mktempdir() | ||
| tmp_project = projectfile_path(tmp) | ||
| tmp_manifest = manifestfile_path(tmp) | ||
| tmp_preferences = joinpath(tmp, first(Base.preferences_names)) |
There was a problem hiding this comment.
Yes. Base.preferences_names is ("JuliaLocalPreferences.toml", "LocalPreferences.toml"), and Base.collect_preferences loops over that tuple and breaks at the first file that exists — so the first name is the one that actually wins, and writing it means a stray LocalPreferences.toml can't shadow what we put there. It's also verbatim what Pkg.Operations.sandbox does. I've added a comment saying so, since it clearly isn't obvious from the code.
| # Copy the preferences over too, as `Pkg.test` does. We reach for `Pkg.TOML` rather | ||
| # than the `TOML` stdlib because TestEnv supports Julia 1.0, which predates it. |
There was a problem hiding this comment.
but this file is only used in Julia 1.11+
There was a problem hiding this comment.
You're right, that reason is nonsense in a file that only ever loads on 1.11+. Pkg.TOML is still the right call, but for a reason the comment failed to state: TestEnv has one Project.toml shared by all eleven version directories with julia = "1" compat, and TOML only became a stdlib in 1.6 — so listing it in [deps] would make the package unloadable on 1.0–1.5. Pkg is already a dependency and does import TOML internally on every version from 1.8 up, so Pkg.TOML gets us the stdlib module for free. Comment now says that, in all five copies.
| # Copy the preferences over too, as `Pkg.test` does. We reach for `Pkg.TOML` rather | ||
| # than the `TOML` stdlib because TestEnv supports Julia 1.0, which predates it. |
There was a problem hiding this comment.
but this file is only used in 1.12
There was a problem hiding this comment.
Same as above — fixed here too. The justification now points at the shared Project.toml rather than at this file's own version floor.
| # `sandbox_preferences` helper in `src/julia-1.8/common.jl` and its siblings. | ||
| if VERSION >= v"1.8-" | ||
| @testset "preferences.jl" begin | ||
| fixture(name) = joinpath(@__DIR__, "preferences", name) |
There was a problem hiding this comment.
weird indirection, why is this here?
There was a problem hiding this comment.
Gone — inlined to joinpath(@__DIR__, "preferences", "PrefsTopLevel"), which is what test/activate_set.jl already does for the sources/ fixtures.
| # `Pkg` -- and hence TestEnv -- writes to the first of `Base.preferences_names` | ||
| preferences_file(project=Base.active_project()) = | ||
| joinpath(dirname(project), first(Base.preferences_names)) | ||
|
|
||
| # The preferences the sandbox itself carries, read straight off disk so the | ||
| # assertion does not depend on whatever `LOAD_PATH` happens to look like. | ||
| sandbox_preferences(name) = Pkg.TOML.parsefile(preferences_file())[name] |
There was a problem hiding this comment.
weird indirections, why are these here?
There was a problem hiding this comment.
Also gone, and they were worse than merely redundant: preferences_file's project parameter was never once passed, and the local sandbox_preferences shadowed the name of the real function in common.jl while meaning something different. Each testset now spells out prefs_file = joinpath(dirname(Base.active_project()), first(Base.preferences_names)) and reads it with Pkg.TOML.parsefile. The three UUID constants were used exactly once each, so they went the same way.
Finishes the work started in #97 (whose commit is cherry-picked here, so @NHDaly keeps authorship of it) and closes #59.
Pkg.testcopies the merged preferences into the sandbox it builds; TestEnv did not, so packages whose tests are configured through Preferences.jl pass under]testbut not underTestEnv.activate(). This ports Pkg's logic across.What changed on top of #97
src/julia-1.9only, which covers Julia 1.9–1.10. The lookup now lives incommon.jlassandbox_preferencesand is applied injulia-1.8,julia-1.9,julia-1.11,julia-1.12andjulia-1.13.do-block form is fixed too.TestEnv.activate(f, pkg)delegates toPkg.Operations.sandbox, which has taken apreferences=keyword since Julia 1.8 but was never passed one — so that form dropped preferences even with the set form fixed.Base.get_preferences()returns an emptyDict, notnothing, so the!== nothingcheck always fired and always wrote an empty preferences file. It now guards on!isempty.source_pathlocal that shadowedPkg.Operations.source_path, qualifiedcopy!(Base.LOAD_PATH, ...)in the restore path, and answered the# TODO: should we separately import TOML?in a comment — no, TestEnv supports Julia 1.0, which predates theTOMLstdlib, soPkg.TOMLit is.Why not Julia 1.7 and earlier
Base.get_preferencesonly grew its no-argument, whole-set form in Julia 1.8; before that it (andcollect_preferences) is scoped to a single package UUID, so there is no way to collect the full merged set to copy across without reimplementing Base's merge.Pkg.Operations.sandboxlikewise only gained itspreferences=keyword in 1.8. Older versions keep today's behaviour, and the README now says so.Tests
Three offline fixture packages under
test/preferences/— no registry dependencies, so they instantiate without network:PrefsTopLevel— preferences at the package root, notest/Project.tomlPrefsTestDir— atest/environment whose preferences shadow the package root's, with one key only set at the root to check the merge orderPrefsNone— no preferences at all, to cover theisemptyguardBoth
activateforms are covered. Verified that all of these fail onmainand pass here.Drive-by fix
test/activate_set.jl's precompile testset guarded itsfinallywithisdefined(@__MODULE__, :orig_load_path)— those are locals inside the testset, not module globals, so the guard was never true andLOAD_PATH/DEPOT_PATHleaked into every later testset. Restoring them unconditionally is needed for the new tests to mean anything.Verification
Full suite run locally against Julia 1.6.7, 1.8.5, 1.9.4, 1.11.9, 1.12.7 and 1.13.0-rc3 — all pass.
🤖 Generated with Claude Code