From 5857c9c24099ffb3483d1954d70ac8daa3dd1747 Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Thu, 25 Jun 2026 07:48:38 -0400 Subject: [PATCH 1/3] QA: add run_qa v1.6 QA group with ExplicitImports enabled Add a `test/qa` group wired through the SciMLTesting run_qa v1.6 harness, running Aqua, JET, and the full ExplicitImports check suite in its own `test/qa` sub-environment (resolves SciMLTesting 1.6.0 from the registry). This supersedes the hand-rolled `test/miscellaneous_tests/explicit_imports.jl` (3 EI checks), which is removed along with the now-unused root ExplicitImports test dependency. Fixes surfaced by enabling QA: - `import Base: (==)` -> `import Base: ==` (the redundant parentheses were the sole JET top-level error; JET now reports 0). - `export CartesianGridReJ` -> `CartesianGridRej` (typo'd, undefined export). - Add [compat] for the stdlibs LinearAlgebra and SparseArrays. Tracked-broken (SciML/Catalyst.jl#1496), preserved via run_qa markers: - aqua_broken: ambiguities (55), unbound_args (GridLattice union leaves T unbound), undefined_exports (`Variable` reexported from ModelingToolkitBase but undefined upstream). - ei_broken: all_qualified_accesses_via_owners, all_qualified_accesses_are_public, all_explicit_imports_are_public (pervasive use of upstream internal/non-public names). The three passing EI checks (no_implicit_imports, no_stale_explicit_imports, all_explicit_imports_via_owners) run as hard tests; the first two pass `allow_unanalyzable = (Catalyst, Catalyst.PhysicalScale)`. deps_compat runs with `check_extras = false` (test-only extras carry no declared bounds). QA group: 11 pass, 6 broken, 0 fail/error on Julia 1.10 and 1.11. Co-Authored-By: Chris Rackauckas Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/Test.yml | 1 + Project.toml | 6 ++-- src/Catalyst.jl | 4 +-- test/miscellaneous_tests/explicit_imports.jl | 17 ---------- test/qa/Project.toml | 10 ++++++ test/qa/qa.jl | 35 ++++++++++++++++++++ test/runtests.jl | 15 ++++++++- 7 files changed, 65 insertions(+), 23 deletions(-) delete mode 100644 test/miscellaneous_tests/explicit_imports.jl create mode 100644 test/qa/Project.toml create mode 100644 test/qa/qa.jl diff --git a/.github/workflows/Test.yml b/.github/workflows/Test.yml index 8d001fbfda..c41f1f551b 100644 --- a/.github/workflows/Test.yml +++ b/.github/workflows/Test.yml @@ -32,6 +32,7 @@ jobs: - Hybrid - Misc - Spatial + - QA uses: "SciML/.github/.github/workflows/tests.yml@v1" with: julia-version: "${{ matrix.version }}" diff --git a/Project.toml b/Project.toml index ede0c1f3ae..9a27264231 100644 --- a/Project.toml +++ b/Project.toml @@ -54,13 +54,13 @@ DocStringExtensions = "0.8, 0.9" DynamicPolynomials = "0.6" DynamicQuantities = "1" EnumX = "1" -ExplicitImports = "1.15" GraphMakie = "0.5, 0.6" Graphs = "1.4" HomotopyContinuation = "2.9" JumpProcesses = "9.23" LaTeXStrings = "1.3.0" Latexify = "0.16.6" +LinearAlgebra = "1.10" MacroTools = "0.5.5" Makie = "0.22.1, 0.23, 0.24" ModelingToolkitBase = "1.17" @@ -77,6 +77,7 @@ RuntimeGeneratedFunctions = "0.5.12" SciMLBase = "2.84, 3" SciMLPublic = "1" Setfield = "1" +SparseArrays = "1.10" StochasticDiffEq = "6.101, 7" StructuralIdentifiability = "0.5.11" SymbolicIndexingInterface = "0.3" @@ -89,7 +90,6 @@ DataInterpolations = "82cc6244-b520-54b8-b5a6-8a565e85f1d0" DiffEqCallbacks = "459566f4-90b8-5000-8ac3-15dfb0a30def" DiffEqNoiseProcess = "77a26b50-5914-5dd7-bc55-306e6241c503" DomainSets = "5b8099bc-c8ec-5219-889f-1d9e522a28bf" -ExplicitImports = "7d51a73a-1435-4ff3-83d9-f097790105c7" LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" Logging = "56ddb016-857b-54e1-b83d-db4d58db5568" NonlinearSolve = "8913a72c-1f9b-4ce2-8d82-65094dcecaec" @@ -112,7 +112,7 @@ Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [targets] test = ["DataInterpolations", "DiffEqCallbacks", "DiffEqNoiseProcess", "DomainSets", - "ExplicitImports", "Logging", "NonlinearSolve", "OrdinaryDiffEqBDF", "OrdinaryDiffEqCore", "OrdinaryDiffEqDefault", + "Logging", "NonlinearSolve", "OrdinaryDiffEqBDF", "OrdinaryDiffEqCore", "OrdinaryDiffEqDefault", "OrdinaryDiffEqRosenbrock", "OrdinaryDiffEqTsit5", "OrdinaryDiffEqVerner", "Pkg", "Plots", "Random", "SafeTestsets", "StableRNGs", "StaticArrays", "Statistics", "SteadyStateDiffEq", "StochasticDiffEq", "Test"] diff --git a/src/Catalyst.jl b/src/Catalyst.jl index 2339490f58..65542edd63 100644 --- a/src/Catalyst.jl +++ b/src/Catalyst.jl @@ -48,7 +48,7 @@ import ModelingToolkitBase: check_variables, check_parameters, # Import from owner module (SymbolicUtils) per ExplicitImports.jl audit import SymbolicUtils: _iszero, unwrap -import Base: (==), hash, size, getindex, setindex, isless, Sort.defalg, length, show +import Base: ==, hash, size, getindex, setindex, isless, Sort.defalg, length, show import MacroTools, Graphs using MacroTools: striplines import Graphs: DiGraph, SimpleGraph, SimpleDiGraph, vertices, edges, add_vertices!, nv, ne @@ -200,7 +200,7 @@ export isedgeparameter include("spatial_reaction_systems/discrete_space_reaction_systems.jl") export DiscreteSpaceReactionSystem export spatial_species, vertex_parameters, edge_parameters -export CartesianGrid, CartesianGridReJ # (Implemented in JumpProcesses) +export CartesianGrid, CartesianGridRej # (Implemented in JumpProcesses) export has_cartesian_dspace, has_masked_dspace, has_grid_dspace, has_graph_dspace, grid_dims, grid_size export make_edge_p_values, make_directed_edge_values diff --git a/test/miscellaneous_tests/explicit_imports.jl b/test/miscellaneous_tests/explicit_imports.jl deleted file mode 100644 index b630a9dbba..0000000000 --- a/test/miscellaneous_tests/explicit_imports.jl +++ /dev/null @@ -1,17 +0,0 @@ -# Tests for import hygiene using ExplicitImports.jl -# Ensures Catalyst imports symbols from their owner modules and has no stale imports. -using Catalyst, ExplicitImports, Test - -@testset "Explicit Imports" begin - # Test that there are no implicit imports - # allow_unanalyzable is needed because PhysicalScale enum causes parsing issues - @test check_no_implicit_imports(Catalyst; - allow_unanalyzable=(Catalyst, Catalyst.PhysicalScale)) === nothing - - # Test that there are no stale explicit imports - @test check_no_stale_explicit_imports(Catalyst; - allow_unanalyzable=(Catalyst, Catalyst.PhysicalScale)) === nothing - - # Test that all explicit imports are from owner modules - @test check_all_explicit_imports_via_owners(Catalyst) === nothing -end diff --git a/test/qa/Project.toml b/test/qa/Project.toml new file mode 100644 index 0000000000..81f390a4f5 --- /dev/null +++ b/test/qa/Project.toml @@ -0,0 +1,10 @@ +[deps] +Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595" +Catalyst = "479239e8-5488-4da2-87a7-35f2df7eef83" +JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b" +SciMLTesting = "09d9d899-5365-40a9-917a-5f67fddea283" + +[compat] +Aqua = "0.8" +JET = "0.9, 0.10, 0.11" +SciMLTesting = "1.6" diff --git a/test/qa/qa.jl b/test/qa/qa.jl new file mode 100644 index 0000000000..8d447e5f8d --- /dev/null +++ b/test/qa/qa.jl @@ -0,0 +1,35 @@ +using SciMLTesting, Catalyst, Test +using JET + +# ExplicitImports cannot fully analyze `Catalyst` (the `PhysicalScale` EnumX enum +# parses as an unanalyzable submodule), so the per-module checks are told to allow it. +const EI_ALLOW_UNANALYZABLE = (Catalyst, Catalyst.PhysicalScale) + +run_qa( + Catalyst; + explicit_imports = true, + # Test-only [extras] in the root Project.toml intentionally carry no [compat] + # entries (they are pinned via the resolver, not declared bounds). + aqua_kwargs = (; deps_compat = (; check_extras = false)), + # Pre-existing Aqua findings tracked in SciML/Catalyst.jl#1496: + # - ambiguities: 55 method ambiguities across the API surface + # - unbound_args: GridLattice `Union{Array{Bool,N}, CartesianGridRej{N,T}}` + # methods leave `T` unbound on the Array branch + # - undefined_exports: `Variable` is reexported (via `@reexport using + # ModelingToolkitBase`) but no longer defined upstream + aqua_broken = (:ambiguities, :unbound_args, :undefined_exports), + ei_kwargs = (; + no_implicit_imports = (; allow_unanalyzable = EI_ALLOW_UNANALYZABLE), + no_stale_explicit_imports = (; allow_unanalyzable = EI_ALLOW_UNANALYZABLE), + ), + # Pre-existing ExplicitImports public-API findings tracked in + # SciML/Catalyst.jl#1496: Catalyst pervasively imports/accesses internal (not + # `public`/exported) names of ModelingToolkitBase, Symbolics, SymbolicUtils, + # SciMLBase, DiffEqBase, JumpProcesses, Latexify, Graphs and Base. These go green + # as the upstream packages mark the names public; tracked-broken until then. + ei_broken = ( + :all_qualified_accesses_via_owners, + :all_qualified_accesses_are_public, + :all_explicit_imports_are_public, + ), +) diff --git a/test/runtests.jl b/test/runtests.jl index 6a02c8a964..e3eecdb3f8 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -12,6 +12,12 @@ function activate_extensions_env() Pkg.instantiate() end +function activate_qa_env() + Pkg.activate("qa") + Pkg.develop(PackageSpec(path = dirname(@__DIR__))) + Pkg.instantiate() +end + ### Run Tests ### @time begin if GROUP == "All" || GROUP == "Modeling" @@ -66,11 +72,18 @@ end if GROUP == "All" || GROUP == "Misc" @time @safetestset "ReactionSystem Serialisation" begin include("miscellaneous_tests/reactionsystem_serialisation.jl") end - @time @safetestset "Explicit Imports" begin include("miscellaneous_tests/explicit_imports.jl") end # BROKEN #@time @safetestset "Latexify" begin include("visualisation/latexify.jl") end # https://github.com/SciML/Catalyst.jl/issues/1352 end + # Quality assurance (Aqua, JET, ExplicitImports) via the SciMLTesting harness. + # Runs in its own `test/qa` sub-environment so the heavier QA tooling does not + # constrain the main test environment's resolution. + if GROUP == "QA" + activate_qa_env() + include("qa/qa.jl") + end + if GROUP == "All" || GROUP == "Spatial" # Tests spatial modelling and simulations. @time @safetestset "PDE Systems Simulations" begin include("spatial_modelling/simulate_PDEs.jl") end From ea8fb94728baec889a7423307eae25092f7d8c85 Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Thu, 25 Jun 2026 10:54:57 -0400 Subject: [PATCH 2/3] QA: skip JET check on the Julia 1.13 prerelease JET 0.11.5 crashes with `UndefRefError: access to undefined reference` (in `JET._unique!` / `analyze_and_report_package!`) when analysing under the Julia 1.13.0-rc1 `Compiler.jl`, so the new `test/qa` JET check breaks the `Tests (pre, QA)` lane while passing on lts and 1 (1.10/1.12). JET is a SciMLTesting weak dependency loaded via `using JET` precisely so it can be omitted on Julia versions it does not support yet; guard that load behind `VERSION < v"1.13-"` so Aqua and ExplicitImports still run on the prerelease lane and the JET check returns once a JET release analyses cleanly on 1.13. Tracked in SciML/Catalyst.jl#1496. Verified locally: JET=0.11.5 + Julia 1.13.0-rc1 reproduces the `UndefRefError` crash in `report_package(Catalyst; mode=:typo)`; the guarded `test/qa/qa.jl` still runs the full QA green on Julia 1.10 LTS (Quality Assurance | Pass 11 | Broken 6 | Total 17). Co-Authored-By: Chris Rackauckas Co-Authored-By: Claude Opus 4.8 (1M context) --- test/qa/qa.jl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/qa/qa.jl b/test/qa/qa.jl index 8d447e5f8d..ea24183c42 100644 --- a/test/qa/qa.jl +++ b/test/qa/qa.jl @@ -1,5 +1,13 @@ using SciMLTesting, Catalyst, Test -using JET + +# JET is a SciMLTesting weak dependency: `using JET` registers it and turns the JET +# check on. JET 0.11 crashes (UndefRefError in `collect_callee_reports!`) when run +# under the Julia 1.13 prerelease `Compiler.jl`, so only load it on the Julia versions +# JET supports. Aqua and ExplicitImports still run on every version. Re-enable JET on +# 1.13 once a JET release analyses cleanly there. Tracked in SciML/Catalyst.jl#1496. +@static if VERSION < v"1.13-" + using JET +end # ExplicitImports cannot fully analyze `Catalyst` (the `PhysicalScale` EnumX enum # parses as an unanalyzable submodule), so the per-module checks are told to allow it. From 82a5f0b77e5374b0807768adbcea617c200da26a Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Mon, 29 Jun 2026 06:39:42 -0400 Subject: [PATCH 3/3] QA: mark pre-existing JET typo-mode findings broken on Julia 1.12+ The new run_qa JET check passes on the lts lane (JET 0.9) but fails on the Julia 1 lane (JET 0.11 on Julia 1.12), which reports 5 pre-existing latent findings that JET 0.9 does not flag and that the old qa.jl never ran (Catalyst had no JET check). All 5 reproduce byte-identical on master: - metadata_entries (src/dsl.jl:436) and reaction (src/chemistry_functionality.jl:400): wrong variable names interpolated into error(...) strings on already-error paths. - clipboard x2 (src/latexify_recipes.jl:65, 161): clipboard (InteractiveUtils) is not imported; only reachable when Latexify.COPY_TO_CLIPBOARD is set. - V (src/reactionsystem.jl:78): the unparameterised NetworkProperties() @kwdef keyword constructor references the type parameter V; that bare constructor is never called (only NetworkProperties{Int, SymbolicT}() is). Gate jet_broken on VERSION >= 1.12 so the JET check stays a hard test_package on lts (where JET reports nothing and jet_broken would flip it to an Unexpected Pass) and becomes a documented @test_broken on the lanes whose JET surfaces the findings. Tracked in SciML/Catalyst.jl#1496. Verified locally on Julia 1.12.6 (released SciMLTesting 1.7.0, JET 0.11.5): Quality Assurance 10 pass / 7 broken / 0 fail / 0 error (was 10 pass / 1 fail / 6 broken). lts behaviour is unchanged (jet_broken evaluates to false = the prior default). Co-Authored-By: Chris Rackauckas --- test/qa/qa.jl | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/qa/qa.jl b/test/qa/qa.jl index ea24183c42..ec58974985 100644 --- a/test/qa/qa.jl +++ b/test/qa/qa.jl @@ -13,6 +13,15 @@ end # parses as an unanalyzable submodule), so the per-module checks are told to allow it. const EI_ALLOW_UNANALYZABLE = (Catalyst, Catalyst.PhysicalScale) +# JET typo-mode surfaces 5 pre-existing latent findings (see the `jet_broken` comment +# below). They are only reported by JET 0.11 (the version resolved on Julia 1.12+); the +# lts lane resolves JET 0.9, which reports nothing, so the JET check must stay a hard +# (un-broken) `test_package` there. `jet_broken` is therefore only enabled on the Julia +# versions whose JET surfaces the findings, otherwise the lts lane would flip to an +# Unexpected Pass. Gating on `VERSION` (not on a JET version query) keeps this a +# compile-time `@static` choice that matches the JET-load gate above. +const JET_BROKEN = VERSION >= v"1.12-" + run_qa( Catalyst; explicit_imports = true, @@ -26,6 +35,24 @@ run_qa( # - undefined_exports: `Variable` is reexported (via `@reexport using # ModelingToolkitBase`) but no longer defined upstream aqua_broken = (:ambiguities, :unbound_args, :undefined_exports), + # Pre-existing JET typo-mode findings tracked in SciML/Catalyst.jl#1496. They are + # latent on master (reproduce byte-identical on the unmodified base) and were never + # caught before because Catalyst had no JET check. JET surfaces them only on the + # Julia 1 lane (JET 0.11 on Julia 1.12); the lts lane runs JET 0.9 and does not + # flag them. `report_package(Catalyst; mode = :typo)` reports 5 names: + # - `metadata_entries` (src/dsl.jl:436) and `reaction` + # (src/chemistry_functionality.jl:400): wrong variable names interpolated into + # `error(...)` strings on already-error paths (should be `metadata_i` / `rx`). + # - `clipboard` x2 (src/latexify_recipes.jl:65, 161): `clipboard` (InteractiveUtils) + # is not imported; only reachable when `Latexify.COPY_TO_CLIPBOARD` is set. + # - `V` (src/reactionsystem.jl:78): the unparameterised `NetworkProperties()` + # `@kwdef` keyword constructor references the type parameter `V` in its + # `Set{V}()`/`Dict{V,Int}()` defaults; that bare constructor is never called + # (only `NetworkProperties{Int, SymbolicT}()` is), so it is a latent dead path. + # `jet_broken` runs `report_package` and asserts the reports are non-empty, so it + # auto-flags an Unexpected Pass once the source typos are fixed and the bare + # constructor is removed/parameterised. + jet_broken = JET_BROKEN, ei_kwargs = (; no_implicit_imports = (; allow_unanalyzable = EI_ALLOW_UNANALYZABLE), no_stale_explicit_imports = (; allow_unanalyzable = EI_ALLOW_UNANALYZABLE),