Skip to content

Commit b3ff68b

Browse files
Make the QA group scan the package extensions (#4860)
ExplicitImports reads the `[extensions]` table but skips any extension whose module does not exist, and an extension module only exists once its trigger package is loaded. The QA environment loaded no weakdeps, so none of the four extensions were ever checked. Add the weakdeps to `test/qa/Project.toml`, load them in `test/qa/qa.jl`, and assert the extension modules actually exist -- ExplicitImports silently skips an extension that fails to load, so a broken extension would otherwise drop coverage to zero while QA stayed green. Fix the findings this exposes in the extension sources: * the three OrdinaryDiffEq precompile extensions relied on implicit imports from their trigger package and PrecompileTools; * `MTKFMIExt` relied on implicit imports from SymbolicIndexingInterface and DocStringExtensions, and reached `setinput`, `setoutput`, `default_toterm` and `unwrap` through ModelingToolkit rather than through ModelingToolkitBase and SymbolicUtils which own them; * `MTKFMIExt`'s `t`/`D` aliases move from a renaming import to `const`, since ExplicitImports tracks the pre-rename name and reports such an import as stale. `FMIComponent`, `Base.RefValue` and the three ModelingToolkitBase metadata helpers have no public spelling, so they are ignored with a comment naming the owner. Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
1 parent 40c9fb5 commit b3ff68b

6 files changed

Lines changed: 69 additions & 15 deletions

File tree

ext/MTKFMIExt.jl

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
module MTKFMIExt
22

33
using ModelingToolkit
4-
using SymbolicIndexingInterface
5-
using ModelingToolkit: t_nounits as t, D_nounits as D
6-
using DocStringExtensions
4+
using SymbolicIndexingInterface: NotSymbolic, symbolic_type
5+
using ModelingToolkitBase: t_nounits, D_nounits
6+
using DocStringExtensions: TYPEDEF, TYPEDFIELDS, TYPEDSIGNATURES
77
import ModelingToolkit as MTK
8+
import ModelingToolkitBase as MTKBase
89
import SciMLBase
10+
import SymbolicUtils
911
import FMIImport as FMI
1012

13+
# Spelled as `const` rather than `t_nounits as t`, because ExplicitImports reports a
14+
# renaming import as stale (it tracks the pre-rename name, which never appears in use).
15+
const t = t_nounits
16+
const D = D_nounits
17+
1118
"""
1219
$(TYPEDSIGNATURES)
1320
@@ -173,7 +180,7 @@ function MTK.FMIComponent(
173180
inputs = []
174181
fmi_variables_to_mtk_variables!(
175182
fmu, FMI.getInputValueReferencesAndNames(fmu),
176-
value_references, inputs, states, observed; postprocess_variable = v -> MTK.setinput(
183+
value_references, inputs, states, observed; postprocess_variable = v -> MTKBase.setinput(
177184
v, true
178185
)
179186
)
@@ -187,7 +194,7 @@ function MTK.FMIComponent(
187194
outputs = []
188195
fmi_variables_to_mtk_variables!(
189196
fmu, FMI.getOutputValueReferencesAndNames(fmu),
190-
value_references, outputs, states, observed; postprocess_variable = v -> MTK.setoutput(
197+
value_references, outputs, states, observed; postprocess_variable = v -> MTKBase.setoutput(
191198
v, true
192199
)
193200
)
@@ -361,22 +368,22 @@ function fmi_variables_to_mtk_variables!(
361368
end
362369
if parameters
363370
vars = [
364-
postprocess_variable(MTK.unwrap(only(@parameters $sname::stateT)))
371+
postprocess_variable(SymbolicUtils.unwrap(only(@parameters $sname::stateT)))
365372
for sname in snames
366373
]
367374
else
368375
vars = [
369-
postprocess_variable(MTK.unwrap(only(@variables $sname(t)::stateT)))
376+
postprocess_variable(SymbolicUtils.unwrap(only(@variables $sname(t)::stateT)))
370377
for sname in snames
371378
]
372379
end
373380
for i in eachindex(vars)
374381
der = ders[i]
375-
vars[i] = MTK.unwrap(vars[i])
382+
vars[i] = SymbolicUtils.unwrap(vars[i])
376383
for j in 1:der
377384
vars[i] = D(vars[i])
378385
end
379-
vars[i] = MTK.default_toterm(vars[i])
386+
vars[i] = MTKBase.default_toterm(vars[i])
380387
end
381388
for i in eachindex(vars)
382389
if i == 1

ext/MTKOrdinaryDiffEqBDFExt.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
module MTKOrdinaryDiffEqBDFExt
22

33
using ModelingToolkit
4-
using OrdinaryDiffEqBDF
5-
using PrecompileTools
4+
using OrdinaryDiffEqBDF: FBDF
5+
using PrecompileTools: @compile_workload, @setup_workload
66
using ModelingToolkit: t_nounits, D_nounits
77

88
@setup_workload begin

ext/MTKOrdinaryDiffEqDefaultExt.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
module MTKOrdinaryDiffEqDefaultExt
22

33
using ModelingToolkit
4-
using OrdinaryDiffEqDefault
5-
using PrecompileTools
4+
using OrdinaryDiffEqDefault: OrdinaryDiffEqDefault
5+
using PrecompileTools: @compile_workload, @setup_workload
66
using ModelingToolkit: t_nounits, D_nounits
77

88
@setup_workload begin

ext/MTKOrdinaryDiffEqRosenbrockExt.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
module MTKOrdinaryDiffEqRosenbrockExt
22

33
using ModelingToolkit
4-
using OrdinaryDiffEqRosenbrock
5-
using PrecompileTools
4+
using OrdinaryDiffEqRosenbrock: Rodas5P
5+
using PrecompileTools: @compile_workload, @setup_workload
66
using ModelingToolkit: t_nounits, D_nounits
77

88
@setup_workload begin

test/qa/Project.toml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
[deps]
22
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
3+
FMIImport = "9fcbc62e-52a0-44e9-a616-1359a0008194"
34
JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
45
ModelingToolkit = "961ee093-0014-501f-94e3-6117800e7a78"
6+
OrdinaryDiffEqBDF = "6ad6398a-0878-4a85-9266-38940aa047c8"
7+
OrdinaryDiffEqDefault = "50262376-6c5a-4cf5-baba-aaf4f84d72d7"
8+
OrdinaryDiffEqRosenbrock = "43230ef6-c299-4910-a778-202eb28ce4ce"
59
SafeTestsets = "1bc83da4-3b8d-516f-aca4-4fe02f6d838f"
610
SciMLTesting = "09d9d899-5365-40a9-917a-5f67fddea283"
711
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
@@ -11,7 +15,11 @@ ModelingToolkit = {path = "../.."}
1115

1216
[compat]
1317
Aqua = "0.8"
18+
FMIImport = "1"
1419
JET = "0.9,0.10,0.11"
20+
OrdinaryDiffEqBDF = "1, 2"
21+
OrdinaryDiffEqDefault = "1.2, 2"
22+
OrdinaryDiffEqRosenbrock = "1, 2"
1523
SafeTestsets = "0.1, 1"
1624
SciMLTesting = "1, 2.1"
1725
Test = "1"

test/qa/qa.jl

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,29 @@ using ModelingToolkit
22
using Aqua
33
using JET
44
using SciMLTesting
5+
using Test
6+
7+
# ExplicitImports only sees an extension module once its trigger package is loaded, so
8+
# every weakdep is loaded here to bring the extensions into the checked module set.
9+
using FMIImport
10+
using OrdinaryDiffEqBDF
11+
using OrdinaryDiffEqDefault
12+
using OrdinaryDiffEqRosenbrock
13+
14+
const MTK_EXTENSIONS = (
15+
:MTKFMIExt,
16+
:MTKOrdinaryDiffEqBDFExt,
17+
:MTKOrdinaryDiffEqDefaultExt,
18+
:MTKOrdinaryDiffEqRosenbrockExt,
19+
)
20+
21+
# ExplicitImports silently skips an extension that fails to load, so assert the
22+
# extension modules actually exist rather than trusting a green run_qa.
23+
@testset "Extensions loaded" begin
24+
for ext in MTK_EXTENSIONS
25+
@test Base.get_extension(ModelingToolkit, ext) !== nothing
26+
end
27+
end
528

629
# Public names that reach ModelingToolkit's API surface only through
730
# `@reexport using Symbolics` in ModelingToolkitBase. They are owned (and undocumented) by
@@ -18,11 +41,27 @@ const SYMBOLICS_OWNED_REEXPORTS = (
1841
:supremum,
1942
)
2043

44+
# Names the extensions must reach for which no public spelling exists.
45+
const NONPUBLIC_QUALIFIED_ACCESSES = (
46+
# ModelingToolkitBase: the FMI extension's variable metadata helpers.
47+
:default_toterm,
48+
:setinput,
49+
:setoutput,
50+
# ModelingToolkit's own stub that MTKFMIExt implements; it is documented API but has
51+
# not been declared `public`, and an extension has no way to add a method unqualified.
52+
:FMIComponent,
53+
# Base: `Base.RefValue` has no public spelling.
54+
:RefValue,
55+
)
56+
2157
run_qa(
2258
ModelingToolkit;
2359
Aqua = Aqua,
2460
JET = JET,
2561
jet = true,
2662
jet_kwargs = (; target_defined_modules = true),
63+
ei_kwargs = (;
64+
all_qualified_accesses_are_public = (; ignore = NONPUBLIC_QUALIFIED_ACCESSES),
65+
),
2766
api_docs_kwargs = (; ignore = SYMBOLICS_OWNED_REEXPORTS),
2867
)

0 commit comments

Comments
 (0)