Repository navigation
Add QA tests with Aqua, ExplicitImports and JET - #299
Merged
Merged
Conversation
devmotion
force-pushed
the
dmw/qa-tests
branch
from
September 18, 2026 12:54
86c286d to
0dbb64b
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #299 +/- ##
=======================================
Coverage 94.29% 94.29%
=======================================
Files 19 19
Lines 1699 1699
=======================================
Hits 1602 1602
Misses 97 97 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Checks that do not pass yet are marked with `@test_broken`. JET >= 0.11.4 requires Julia >= 1.12, so no single version resolves across the whole CI matrix. The compat entry allows JET 0.9.18 as well, and the checks are only run if `JET_AVAILABLE` is defined and true. Since the downgrade job would otherwise pin JET to 0.9.18 on Julia >= 1.12, JET is added to its skip list. `report_package` reports hundreds of problems in code generated with `@eval`, hence the API entry points are analyzed individually instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The six `@test_broken` ExplicitImports checks become real tests. Four are fixed at the source, two are narrowed to what the package can control. Source fixes: - `src/ReverseDiff.jl`: all imports are explicit now, which also drops the stale `ForwardDiff.Partials`. The bare module bindings `DiffRules`, `NaNMath` and `SpecialFunctions` are kept: the `@eval` loops in `derivatives/` splice `$M.$f` for `M in (:Base, :SpecialFunctions, :NaNMath)` and iterate `DiffRules.diffrules(; filter_modules=nothing)`. - `src/derivatives/linalg/special.jl`: `inv` is owned by `Base`, not by `LinearAlgebra`, so extend `Base.inv`. (`LinearAlgebra.det` above it is correct as-is -- `det` *is* owned by `LinearAlgebra`.) - `src/derivatives/linalg/arithmetic.jl`: drop the self-qualification in `Base.zero(x::ReverseDiff.TrackedArray)`. - `src/macros.jl`: interpolate the module into the code that `@grad_from_chainrules` generates, exactly as `@grad` already does. This removes the remaining 12 self-qualified accesses and, because `ChainRulesCore` is interpolated too, the generated code no longer requires the caller's module to have `ChainRulesCore` in scope. Test changes: The two publicness checks are scoped with `ignore`. ReverseDiff cannot mark its own names `public` -- that needs Julia >= 1.11 and the LTS is 1.10 -- and several dependencies predate the keyword, so the listed names are all either upstream internals or ReverseDiff internals used by `StatisticsExt`. ExplicitImports 1.15 has no way to say "this extension belongs to this package": an extension is a top-level module, so `Base.moduleroot` of `StatisticsExt` is itself rather than `ReverseDiff` and `allow_internal_imports`/`allow_internal_accesses` never apply, while `ignore=(STATISTICS_EXT,)` is rejected as not a submodule. Listing the names is the only way to express it, and it keeps the extension's dependence on internals visible. `check_all_explicit_imports_via_owners` gets `StaticArrays => StaticArraysCore` in `skip`, since `SVector` is exported by `StaticArrays` but owned by `StaticArraysCore`; that adds `StaticArraysCore` to the test dependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- `src/ReverseDiff.jl`: import the `LinearAlgebra` and `StaticArrays` names that
the code uses now, e.g. `Diagonal` and `StaticArray`.
- `src/derivatives/broadcast.jl`: drop the stale `BroadcastStyle` import, which
is only used qualified, and assert that `holdsperturbations` returns a `Bool`.
`all` of an abstract `Tuple` infers as `Union{Missing, Bool}`, so JET reported
a non-boolean condition in `copy(::Broadcasted{<:TrackedStyle})` for every
`jacobian` of a broadcasting function.
- `test/QATests.jl`: drop the module wrapper and the `runtests.jl` ordering
comment, since ParallelTestRunner runs every file in its own worker, define
the test functions at file scope, and update the non-public names. The
publicness checks only run on Julia >= 1.11: Julia 1.10 has no `public`
keyword, so ExplicitImports reports e.g. `Base.tail` there.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devmotion
force-pushed
the
dmw/qa-tests
branch
from
October 25, 2026 21:34
005f56d to
67959be
Compare
devmotion
marked this pull request as ready for review
October 25, 2026 21:47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
QATeststest set that runs Aqua, ExplicitImports and JET.Changes
src/ReverseDiff.jl, and fixes for the other ExplicitImports findings:Base.invinstead ofLinearAlgebra.inv, no self-qualified accesses, a staleBroadcastStyleimport.@grad_from_chainrulesnow interpolatesReverseDiffandChainRulesCoreinto the code it generates, like@gradalready does.holdsperturbationsinsrc/derivatives/broadcast.jlasserts::Boolon the result ofall. For an abstractTuple,allinfers asUnion{Missing, Bool}, so JET reported a non-boolean condition incopy(::Broadcasted{<:TrackedStyle})for everyjacobianof a broadcasting function.Checks marked broken
Aqua.test_ambiguities: methods generated with@evalclash withBaseandLinearAlgebra: 551 ambiguities on Julia 1.12, 562 on 1.13, 1809 on 1.10.pull_value!(::TrackedReal{V, D, Nothing})insrc/tracked.jl, wheret.originis non-nothingonly because of the run-time checkhasorigin(t).@test_opton uncompiled tapes is also broken, because they dispatch onAbstractInstructionat run time.Version notes
publickeyword, so it reports names such asBase.tailthat later versions mark public.JET_AVAILABLEis defined and true. JET is added to the skip list of the downgrade job, which would otherwise pin it to 0.9.18 on Julia >= 1.12, where it does not precompile.Testing
QATestspasses locally on Julia 1.12.7 and 1.13.1 (run directly) and on 1.10.12 (viaPkg.test(; test_args=["QATests"])). The rest of the suite and the downgrade job were not run locally.🤖 Generated with Claude Code