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..ec58974985 --- /dev/null +++ b/test/qa/qa.jl @@ -0,0 +1,70 @@ +using SciMLTesting, Catalyst, Test + +# 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. +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, + # 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), + # 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), + ), + # 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