Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Aqua tests #662

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,28 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
ForwardDiffStaticArraysExt = "StaticArrays"

[compat]
Aqua = "0.8"
Calculus = "0.5"
CommonSubexpressions = "0.3"
DiffResults = "1.1"
DiffRules = "1.4"
DiffTests = "0.1"
InteractiveUtils = "1"
LinearAlgebra = "1"
IrrationalConstants = "0.1, 0.2"
LogExpFunctions = "0.3"
NaNMath = "1"
Preferences = "1"
Printf = "1"
Random = "1"
SparseArrays = "1"
SpecialFunctions = "1, 2"
StaticArrays = "1.5"
Test = "1"
julia = "1.6"

[extras]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
Calculus = "49dc2e85-a5d0-5ad3-a950-438e2897f1b9"
DiffTests = "de460e47-3fe3-5279-bb4a-814414816d5d"
InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240"
Expand All @@ -45,4 +53,4 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["Calculus", "DiffTests", "IrrationalConstants", "SparseArrays", "StaticArrays", "Test", "InteractiveUtils"]
test = ["Aqua", "Calculus", "DiffTests", "IrrationalConstants", "SparseArrays", "StaticArrays", "Test", "InteractiveUtils"]
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

[![](https://img.shields.io/badge/docs-stable-blue.svg)](https://juliadiff.org/ForwardDiff.jl/stable)
[![](https://img.shields.io/badge/docs-dev-blue.svg)](https://juliadiff.org/ForwardDiff.jl/dev)
[![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)

# ForwardDiff.jl

Expand Down
41 changes: 41 additions & 0 deletions test/AquaTest.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
module AquaTest

using Test
using ForwardDiff
using Aqua

@testset "Aqua tests - unbound_args" begin
# This tests that we don't accidentally run into
# https://github.com/JuliaLang/julia/issues/29393
ua = Aqua.detect_unbound_args_recursively(ForwardDiff)
@test length(ua) == 6
end
Comment on lines +7 to +12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the unbound arguments should be fixed, which would also allow us to just run the default Aqua.test_all test with unbound_args = true below instead of this custom test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These ambiguities seem to be upstream issues. See JuliaTesting/Aqua.jl#86 and JuliaLang/julia#28086.

We can "fix" this (I'm not 100% sure if the issue is here or upstream, even based on discussions) by changing e.g.,

@generated function one_tuple(::Type{NTuple{N,V}}) where {N, V}

to

@generated function one_tuple(::Type{Tuple{V,Vararg{R1,N}}) where {N, V}

Should I move forward with those changes?


@testset "Aqua tests - ambiguities" begin
# See: https://github.com/SciML/OrdinaryDiffEq.jl/issues/1750
# Test that we're not introducing method ambiguities across deps
ambs = Aqua.detect_ambiguities(ForwardDiff; recursive = true)
pkg_match(pkgname, pkdir::Nothing) = false
pkg_match(pkgname, pkdir::AbstractString) = occursin(pkgname, pkdir)
filter!(x -> pkg_match("ForwardDiff", pkgdir(last(x).module)), ambs)

ambs_dict = Dict()
ambs_dict[(1, 6)] = 2
ambs_dict[(1, 10)] = 1
verkey(v) = (Int(VERSION.major), Int(VERSION.minor))

if haskey(ambs_dict, verkey(VERSION))
@test length(ambs) ≤ ambs_dict[verkey(VERSION)]
# notify us when we fix one
if length(ambs) < ambs_dict[verkey(VERSION)]
@info "Ambiguities may have been fixed, please lower the limit."
@info " length(ambs) = $(length(ambs))"
end
end
end

@testset "Aqua tests - remaining" begin
Aqua.test_all(ForwardDiff; ambiguities = false, unbound_args = false)
end

end
5 changes: 5 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Random.seed!(SEED)

@testset "ForwardDiff.jl" begin
t0 = time()
@testset "Aqua" begin
println("##### Testing aqua...")
t = @elapsed include("AquaTest.jl")
println("##### done (took $t seconds).")
end
@testset "Partials" begin
println("##### Testing Partials...")
t = @elapsed include("PartialsTest.jl")
Expand Down
Loading