Copy preferences into the test environment - #121
Open
davidanthoff wants to merge 7 commits into
Open
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:
|
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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