Skip to content

Add QA tests with Aqua, ExplicitImports and JET - #299

Merged
devmotion merged 3 commits into
masterfrom
dmw/qa-tests
Oct 26, 2026
Merged

devmotion merged 3 commits into
masterfrom
dmw/qa-tests

Conversation

@devmotion

@devmotion devmotion commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Adds a QATests test set that runs Aqua, ExplicitImports and JET.

Changes

  • Explicit imports in src/ReverseDiff.jl, and fixes for the other ExplicitImports findings: Base.inv instead of LinearAlgebra.inv, no self-qualified accesses, a stale BroadcastStyle import. @grad_from_chainrules now interpolates ReverseDiff and ChainRulesCore into the code it generates, like @grad already does.
  • holdsperturbations in src/derivatives/broadcast.jl asserts ::Bool on the result of all. For an abstract Tuple, all infers as Union{Missing, Bool}, so JET reported a non-boolean condition in copy(::Broadcasted{<:TrackedStyle}) for every jacobian of a broadcasting function.

Checks marked broken

  • Aqua.test_ambiguities: methods generated with @eval clash with Base and LinearAlgebra: 551 ambiguities on Julia 1.12, 562 on 1.13, 1809 on 1.10.
  • JET reports every entry point that differentiates a function with a scalar output. The analysis reaches pull_value!(::TrackedReal{V, D, Nothing}) in src/tracked.jl, where t.origin is non-nothing only because of the run-time check hasorigin(t). @test_opt on uncompiled tapes is also broken, because they dispatch on AbstractInstruction at run time.

Version notes

  • The ExplicitImports publicness checks only run on Julia >= 1.11. Julia 1.10 has no public keyword, so it reports names such as Base.tail that later versions mark public.
  • JET >= 0.11.4 requires Julia >= 1.12, so no single version resolves on all Julia versions in the CI matrix. The compat entry therefore also allows JET 0.9.18, and the checks only run if JET_AVAILABLE is 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

QATests passes locally on Julia 1.12.7 and 1.13.1 (run directly) and on 1.10.12 (via Pkg.test(; test_args=["QATests"])). The rest of the suite and the downgrade job were not run locally.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.29%. Comparing base (51cda83) to head (67959be).

Files with missing lines Patch % Lines
src/derivatives/linalg/arithmetic.jl 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

devmotion and others added 3 commits October 25, 2026 22:33
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
devmotion marked this pull request as ready for review October 25, 2026 21:47
@devmotion
devmotion merged commit 0108ea1 into master Oct 26, 2026
8 of 9 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.

1 participant