From 8280a8960979a5b2e7a8dcefce7c11d272c77b27 Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Mon, 20 Jul 2026 21:32:25 -0400 Subject: [PATCH] Enable public reexport QA by default Co-Authored-By: Chris Rackauckas --- Project.toml | 2 +- README.md | 12 ++++++------ src/SciMLTesting.jl | 18 +++++++++--------- test/runtests.jl | 14 ++++++++------ 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/Project.toml b/Project.toml index 213cdf3..7732561 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "SciMLTesting" uuid = "09d9d899-5365-40a9-917a-5f67fddea283" authors = ["SciML"] -version = "2.4.0" +version = "2.5.0" [deps] Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595" diff --git a/README.md b/README.md index a716849..1d423ac 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ test = ["Test", "SciMLTesting", ...] | `current_group(; env = "GROUP", default = "All")` | Read the test-group env var, defaulting to `"All"` (empty string also normalizes to the default). | | `activate_group_env(group_dir; parent, develop, instantiate, develop_sources)` | `Pkg.activate` a per-group `Project.toml`, `develop` the parent package(s) by path, backport `[sources]`, `instantiate`. | | `develop_sources!(group_dir; parent)` | On Julia < 1.11, `Pkg.develop` the env's `[sources]` path graph (recursively); a no-op on 1.11+. | -| `run_qa(pkg; Aqua, JET, ExplicitImports, aqua, jet, explicit_imports, api_docs, check_reexports, aqua_broken, jet_broken, ei_broken, ...)` | Run the standard Aqua/JET/ExplicitImports QA body, plus the public-API documentation check. Aqua and ExplicitImports run by default; `using JET` registers JET via its weakdep extension and turns the JET check on. `api_docs` is **on by default** and runs `run_api_docs`. `check_reexports` is opt-in and rejects public dependency bindings except `reexports_allow`. The `*_broken` kwargs mark known-broken findings as `@test_broken`. | +| `run_qa(pkg; Aqua, JET, ExplicitImports, aqua, jet, explicit_imports, api_docs, check_reexports, aqua_broken, jet_broken, ei_broken, ...)` | Run the standard Aqua/JET/ExplicitImports QA body, public-API documentation check, and public-reexport audit. Aqua, ExplicitImports, API docs, and `check_reexports` run by default; `using JET` registers JET and turns its check on. Intentional facade bindings must be listed in `reexports_allow`. The `*_broken` kwargs mark known-broken findings as `@test_broken`. | | `run_api_docs(pkg; docstrings = true, rendered = true, docs_src, ignore, rendered_ignore, docstrings_broken, rendered_broken)` | Assert every exported/`public` name of `pkg` has a docstring and every locally rendered name appears in a `@docs` block under `docs/src`. Re-exported dependency modules inherit their defining package's rendering. | | `public_api_names(pkg)` | The sorted public API of `pkg` (exported names, plus `public` names on Julia ≥ 1.11), with the module's own name dropped. | | `public_reexports(pkg; allow = ())` | Public names imported from outside the package module hierarchy, plus aliases with reflectable external module ownership, excluding intentional names in `allow`. | @@ -351,14 +351,14 @@ run_api_docs(MyPackage; rendered = false) # docstrings only On the Julia 1.10 LTS `public_api_names` returns only the exported names (the `public` keyword is 1.11+), so no per-repo `if VERSION` guards are needed. -### Public reexport audit (`check_reexports = true`) +### Public reexport audit -The reexport audit is opt-in so adopting this SciMLTesting release does not immediately -break existing facade packages. Enable it while cleaning each package's public surface: +The reexport audit runs by default. Packages that deliberately provide a facade API must +list those bindings explicitly: ```julia -run_qa(MyPackage; check_reexports = true) -run_qa(MyFacade; check_reexports = true, reexports_allow = (:solve, :remake)) +run_qa(MyPackage) +run_qa(MyFacade; reexports_allow = (:solve, :remake)) ``` It detects imported functions, types, modules, scalar constants, macros, and operators. diff --git a/src/SciMLTesting.jl b/src/SciMLTesting.jl index eb3d6de..46d7b5c 100644 --- a/src/SciMLTesting.jl +++ b/src/SciMLTesting.jl @@ -609,7 +609,7 @@ end """ run_qa(pkg; Aqua = Aqua, JET = ..., ExplicitImports = ExplicitImports, aqua = Aqua !== nothing, jet = JET !== nothing, - explicit_imports = true, api_docs = true, check_reexports = false, + explicit_imports = true, api_docs = true, check_reexports = true, clean_sources = true, aqua_kwargs = (;), jet_kwargs = (; target_modules = (pkg,), mode = :typo), ei_kwargs = (;), api_docs_kwargs = (;), reexports_allow = (), @@ -646,8 +646,9 @@ QA expectation across the fleet, so it runs for every `run_qa` caller by default document a repo's public API, or curate exceptions via `api_docs_kwargs` (`ignore`, `docstrings_broken`), or pass `api_docs = false` to skip it. `api_docs_kwargs` is forwarded to [`run_api_docs`](@ref) (e.g. `rendered`, `ignore`, `docstrings_broken`). -`explicit_imports` defaults to **`true`**. Packages with unavoidable dependency -exceptions provide their per-check ignore-lists through `ei_kwargs`. Setting an enable +`explicit_imports` and `check_reexports` default to **`true`**. Packages with unavoidable +dependency exceptions provide their per-check ignore-lists through `ei_kwargs`, while +facade packages list intentional public reexports in `reexports_allow`. Setting an enable flag `true` while its module is unavailable is a configuration error and throws an `ArgumentError`. The whole thing runs inside a `@testset` named `testset`. @@ -725,9 +726,8 @@ run_qa(MyPackage) using SciMLTesting, MyPackage run_qa(MyPackage) -# Reject accidental public reexports. Facade packages can allow intentional names: -run_qa(MyPackage; check_reexports = true) -run_qa(MyFacade; check_reexports = true, reexports_allow = (:solve, :remake)) +# Public reexports are rejected by default. Facade packages allow intentional names: +run_qa(MyFacade; reexports_allow = (:solve, :remake)) # Explicitly threading the modules in still works (backward-compatible): using SciMLTesting, Aqua, JET @@ -751,7 +751,7 @@ function run_qa( jet::Bool = JET !== nothing, explicit_imports::Bool = true, api_docs::Bool = true, - check_reexports::Bool = false, + check_reexports::Bool = true, reexports_allow = (), clean_sources::Bool = true, aqua_kwargs = (;), @@ -908,8 +908,8 @@ end public_reexports(pkg; allow = ()) -> Vector{Symbol} Return public names imported from, or aliased to API owned outside, `pkg`'s module -hierarchy. This is an opt-in QA audit: facade packages may allow deliberate reexports, -while ordinary packages should expose only their own API. +hierarchy. [`run_qa`](@ref) runs this audit by default: facade packages allow deliberate +reexports, while ordinary packages should expose only their own API. `allow` is a collection of public names that are intentional reexports. Imported bindings are detected regardless of value kind. Local aliases are also detected when diff --git a/test/runtests.jl b/test/runtests.jl index 950dd81..df3f4e9 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1229,12 +1229,13 @@ end ExplicitImports = nothing, explicit_imports = false, api_docs = false, + check_reexports = false, ) end @test c[:pass] == 0 && c[:fail] == 0 && c[:error] == 0 && c[:broken] == 0 end - @testset "run_qa public reexport integration (opt in)" begin + @testset "run_qa public reexport integration (default on)" begin function count_results(ts) counts = Dict(:pass => 0, :fail => 0, :error => 0, :broken => 0) for r in ts.results @@ -1265,22 +1266,23 @@ end explicit_imports = false, api_docs = false, ) - # Compatibility: the audit is off unless a package explicitly enables it. + # The default path rejects every unapproved public reexport. c = counts_of() do run_qa(ComprehensiveReexportFixture; qa_kwargs...) end - @test c[:pass] == 0 && c[:fail] == 0 && c[:error] == 0 + @test c[:fail] == 1 && c[:error] == 0 + # The enable flag still has an explicit diagnostic escape hatch. c = counts_of() do - run_qa(ComprehensiveReexportFixture; qa_kwargs..., check_reexports = true) + run_qa(ComprehensiveReexportFixture; qa_kwargs..., check_reexports = false) end - @test c[:fail] == 1 && c[:error] == 0 + @test c[:pass] == 0 && c[:fail] == 0 && c[:error] == 0 + # Intentional facade API passes only when every reexport is listed. c = counts_of() do run_qa( ComprehensiveReexportFixture; qa_kwargs..., - check_reexports = true, reexports_allow = (:OwnedModule, :OwnedType, :owned_function, :owned_scalar), ) end