Skip to content

Route QA group through the reserved qa= keyword in run_tests#137

Merged
ChrisRackauckas merged 3 commits into
SciML:mainfrom
ChrisRackauckas-Claude:fix-qa-group-routing
Jun 17, 2026
Merged

Route QA group through the reserved qa= keyword in run_tests#137
ChrisRackauckas merged 3 commits into
SciML:mainfrom
ChrisRackauckas-Claude:fix-qa-group-routing

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Problem

The QA matrix entry of tests CI on main fails for both julia 1 and julia lts:

ERROR: LoadError: ArgumentError: run_tests: GROUP="QA" was requested but no `qa` body was provided
   @ SciMLTesting ~/.julia/packages/SciMLTesting/.../src/SciMLTesting.jl:959
   @ ~/_work/BaseModelica.jl/BaseModelica.jl/test/runtests.jl:20

This was introduced by #134, which rewrote test/runtests.jl to the SciMLTesting v1.2 explicit-args run_tests form but registered the QA group via groups = Dict("QA" => qa_group).

In the explicit-args dispatcher, "QA" (like "Core"/"All") is a reserved group name: when GROUP=QA is requested it is resolved against the qa= keyword and is not looked up in the groups Dict. With qa left unset (default nothing), the reserved-name branch throws.

Fix

Pass the existing qa_group thunk through the reserved qa= keyword instead of groups:

-    groups = Dict("QA" => qa_group),
+    qa = qa_group,

GROUP=QA and the "Quality" umbrella alias both resolve to qa_group; GROUP=Core and the curated GROUP=All (all = ["Core"]) are unchanged. The prerelease no-op guard inside qa_group is preserved.

Local verification

Against the registered SciMLTesting v1.2.0 (the build CI resolved) on Julia 1.10, with a stub thunk to exercise the dispatcher:

=== BROKEN form (current main), GROUP=QA ===
THREW: ArgumentError: run_tests: GROUP="QA" was requested but no `qa` body was provided

=== FIXED form ===
GROUP=Core     => ["CORE"]
GROUP=QA       => ["QA"]
GROUP=Quality  => ["QA"]
GROUP=All      => ["CORE"]

The broken form reproduces the CI error verbatim; the fixed form routes every GROUP correctly. Runic (v1.7) reports test/runtests.jl is already formatted.

Please ignore until reviewed by @ChrisRackauckas.

🤖 Generated with Claude Code


Follow-up: fix the genuine JET report exposed once the QA group runs (Julia 1.12)

With the QA routing fixed above, the QA matrix entry on the julia 1 channel
(now 1.12) actually runs test/qa/test_jet.jl, which exposed a single genuine
report_package MethodError in BaseModelica's own source at
src/evaluator.jl (eval_AST(::BaseModelicaWhenEquation)):

no matching method found `kwcall(::@NamedTuple{affect_neg::Nothing,
  discrete_parameters::Vector{Any}}, ::Type{SymbolicContinuousCallback},
  ::Vector{Vector{Equation}}, ::Vector{Equation})` (1/2 union split)

Root cause: to_zero_crossing/eval_when_rhs return a Union whose vector arm
makes crossing ~ 0 inferred as Union{Equation, Vector{Equation}}, so the
array literal [crossing ~ 0] is Union{Vector{Equation}, Vector{Vector{Equation}}}.
The Vector{Vector{Equation}} arm has no matching SymbolicContinuousCallback
method — a real method error on that inferred path. lts/1.10 didn't exercise
that arm, so it only surfaced on 1.12.

Fix: annotate both SymbolicContinuousCallback zero-crossing literals as
Equation[crossing ~ 0] so they are unambiguously Vector{Equation}. A
when-condition is always scalar, so this is behavior-preserving for valid input
and removes the spurious method path.

Local verification on Julia 1.12.6 against the registered test/qa env: JET
report_package over all 364 top-level definitions now reports 0 errors
("No errors detected"), so test/qa/test_jet.jl:12 passes. Runic v1.7 reports
src/evaluator.jl already formatted.

Please ignore until reviewed by @ChrisRackauckas.

ChrisRackauckas and others added 3 commits June 15, 2026 10:09
The explicit-args run_tests dispatcher treats "QA" as a reserved group name
that is resolved against the qa= keyword, not looked up in the groups Dict.
Registering it via groups = Dict("QA" => qa_group) left qa unset, so a
GROUP=QA matrix entry hit the reserved-name branch and threw
ArgumentError: GROUP="QA" was requested but no qa body was provided.

Pass the qa_group thunk as qa = qa_group. GROUP=QA and the "Quality"
umbrella alias both resolve to it; GROUP=Core / GROUP=All are unchanged.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PID_Controller.bmo test fixture uses the standard Modelica LimPID
derivative-gain parameter `Nd` (both as the parameter name 'PI.Nd' and in
its description string). typos flags `Nd` -> `And`; add it to
default.extend-words as a domain false-positive so spellcheck passes.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`eval_AST(::BaseModelicaWhenEquation)` built the zero-crossing equation
vector as `[crossing ~ 0]`. Because `to_zero_crossing`/`eval_when_rhs`
return a Union that includes a vector arm, `crossing ~ 0` is inferred as
`Union{Equation, Vector{Equation}}`, making `[crossing ~ 0]` an
`Union{Vector{Equation}, Vector{Vector{Equation}}}`. The
`Vector{Vector{Equation}}` arm has no matching `SymbolicContinuousCallback`
method, which JET's report_package flags as a genuine MethodError on the
QA group (Julia 1 = 1.12 channel; this was the single report failing
test/qa/test_jet.jl:12 there, while lts/1.10 did not exercise that arm).

Annotate both SymbolicContinuousCallback zero-crossing literals as
`Equation[crossing ~ 0]` so they are unambiguously `Vector{Equation}`.
A when-condition is always scalar, so this is behavior-preserving for
valid input and removes the spurious method path.

Verified locally on Julia 1.12.6 against the test/qa env: JET
report_package over all 364 top-level definitions now finds 0 reports
("No errors detected"), so test_jet.jl:12 passes. Runic v1.7 reports the
file already formatted.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ChrisRackauckas ChrisRackauckas marked this pull request as ready for review June 17, 2026 10:49
@ChrisRackauckas ChrisRackauckas merged commit 9c98b5d into SciML:main Jun 17, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants