Refactor test_deps_compat

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `test_deps_compat` has a new kwarg `broken` to mark the test as broken using `Test.@test_broken`. ([#193](https://github.com/JuliaTesting/Aqua.jl/pull/193))
+
 ### Fixed
 
 - `test_piracy` no longer prints warnings for methods where the third argument is a `TypeVar`. ([#188](https://github.com/JuliaTesting/Aqua.jl/pull/188))

src/deps_compat.jl

@@ -1,84 +1,74 @@
 """
-    Aqua.test_deps_compat(package; [ignore])
+    Aqua.test_deps_compat(package)
 
-Test that `Project.toml` of `package` list all `compat` for `deps`.
+Test that the `Project.toml` of `package` has a `compat` entry for
+each package listed under `deps`.
 
 # Arguments
 - `packages`: a top-level `Module`, a `Base.PkgId`, or a collection of
   them.
 
 # Keyword Arguments
+- `broken::Bool = false`: If true, it uses `@test_broken` instead of
+  `@test`.
 - `ignore::Vector{Symbol}`: names of dependent packages to be ignored.
 """
-function test_deps_compat(packages; kwargs...)
-    @testset "$(result.label)" for result in analyze_deps_compat(packages; kwargs...)
-        @debug result.label result
-        @test result ⊜ true
+function test_deps_compat(pkg::PkgId; broken::Bool = false, kwargs...)
+    result = find_missing_deps_compat(pkg; kwargs...)
+    if broken
+        @test_broken isempty(result)
+    else
+        @test isempty(result)
     end
 end
 
-function analyze_deps_compat(packages; kwargs...)
-    result = [_analyze_deps_compat_1(pkg; kwargs...) for pkg in aspkgids(packages)]
-    return result
+# Remove with next breaking version
+function test_deps_compat(packages::Vector{<:Union{Module,PkgId}}; kwargs...)
+    @testset "$pkg" for pkg in packages
+        test_deps_compat(pkg; kwargs...)
+    end
+end
+
+function test_deps_compat(mod::Module; kwargs...)
+    test_deps_compat(aspkgid(mod); kwargs...)
 end
 
-function _analyze_deps_compat_1(pkg::PkgId; kwargs...)
+function find_missing_deps_compat(pkg::PkgId, deps_type::String = "deps"; kwargs...)
     result = root_project_or_failed_lazytest(pkg)
-    result isa LazyTestResult && return result
+    result isa LazyTestResult && error("Unable to locate Project.toml")
     root_project_path = result
-    return _analyze_deps_compat_2(
-        pkg,
-        root_project_path,
-        TOML.parsefile(root_project_path);
-        kwargs...,
-    )
+    missing_compat =
+        find_missing_deps_compat(TOML.parsefile(root_project_path), deps_type; kwargs...)
+
+    if !isempty(missing_compat)
+        printstyled(
+            stderr,
+            "$pkg does not declare a compat entry for the following $deps_type:\n";
+            bold = true,
+            color = Base.error_color(),
+        )
+        show(stderr, MIME"text/plain"(), missing_compat)
+        println(stderr)
+    end
+
+    return missing_compat
 end
 
-# For supporting Julia 1.8-DEV and above which give us a tuple instead of a string
-_unwrap_name(x::Tuple) = first(x)
-_unwrap_name(x::String) = x
-_unwrap_name(x::Nothing) = x
-function _analyze_deps_compat_2(
-    pkg::PkgId,
-    root_project_path,
-    prj;
+function find_missing_deps_compat(
+    prj::Dict{String,Any},
+    deps_type::String;
     ignore::AbstractVector{Symbol} = Symbol[],
 )
-    label = "$pkg"
+    deps = get(prj, deps_type, Dict{String,Any}())
+    compat = get(prj, "compat", Dict{String,Any}())
 
-    deps = get(prj, "deps", nothing)
-    if deps === nothing
-        return LazyTestResult(label, "`$root_project_path` does not have `deps`", true)
-    end
-    compat = get(prj, "compat", nothing)
-    if compat === nothing
-        return LazyTestResult(label, "`$root_project_path` does not have `compat`", false)
-    end
-
-    stdlib_name_from_uuid = stdlibs()
-    stdlib_deps = filter!(
-        !isnothing,
+    stdlibs = get_stdlib_list()
+    missing_compat = sort!(
         [
-            _unwrap_name(get(stdlib_name_from_uuid, UUID(uuid), nothing)) for
-            (_, uuid) in deps
-        ],
-    )
-    missing_compat =
-        setdiff(setdiff(setdiff(keys(deps), keys(compat)), stdlib_deps), String.(ignore))
-    if !isempty(missing_compat)
-        msg = join(
-            [
-                "`$root_project_path` does not specify `compat` for:"
-                sort!(collect(missing_compat))
-            ],
-            "\n",
-        )
-        return LazyTestResult(label, msg, false)
-    end
-
-    return LazyTestResult(
-        label,
-        "`$root_project_path` specifies `compat` for all `deps`",
-        true,
+            d for d in map(d -> PkgId(UUID(last(d)), first(d)), collect(deps)) if
+            !(d.name in keys(compat)) && !(d in stdlibs) && !(d.name in String.(ignore))
+        ];
+        by = (pkg -> pkg.name),
     )
+    return missing_compat
 end

src/utils.jl

@@ -84,16 +84,25 @@
     return code
 end
 
-const stdlibs = try
-    Pkg.Types.stdlibs
-catch
-    try
-        # https://github.com/JuliaLang/Pkg.jl/pull/1559
-        Pkg.Types.stdlib  # julia < 1.4
-    catch
-        # https://github.com/JuliaLang/Pkg.jl/pull/696
-        Pkg.Types.gather_stdlib_uuids  # julia < 1.1
+
+
+function get_stdlib_list()
+    @static if VERSION >= v"1.4-"
+        result = Pkg.Types.stdlibs()
+    elseif VERSION >= v"1.1-"
+        result = Pkg.Types.stdlib()
+    else
+        result = Pkg.Types.gather_stdlib_uuids()
+    end
+
+    @static if VERSION >= v"1.8-"
+        # format: Dict{Base.UUID, Tuple{String, Union{Nothing, VersionNumber}}}
+        libs = [PkgId(first(entry), first(last(entry))) for entry in result]
+    else
+        # format Dict{Base.UUID, String}
+        libs = [PkgId(first(entry), last(entry)) for entry in result]
     end
+    return libs
 end
 
 const _project_key_order = [

test/test_deps_compat.jl

@@ -1,82 +1,73 @@
 module TestDepsCompat
 
 include("preamble.jl")
-using Aqua: PkgId, UUID, _analyze_deps_compat_2, ⊜
+using Aqua: find_missing_deps_compat
 
 const DictSA = Dict{String,Any}
 
-@testset "_analyze_deps_compat_2" begin
-    pkg = PkgId(UUID(42), "TargetPkg")
-    root_project_path = "DUMMY_PATH"
+@testset "find_missing_deps_compat" begin
     @testset "pass" begin
-        @test _analyze_deps_compat_2(
-            pkg,
-            root_project_path,
+        result = find_missing_deps_compat(
             DictSA("deps" => DictSA(), "compat" => DictSA("julia" => "1")),
-        ) ⊜ true
-        @test _analyze_deps_compat_2(
-            pkg,
-            root_project_path,
+            "deps",
+        )
+        @test isempty(result)
+        result = find_missing_deps_compat(
             DictSA(
                 "deps" => DictSA("SHA" => "ea8e919c-243c-51af-8825-aaa63cd721ce"),
                 "compat" => DictSA("julia" => "1"),
             ),
-        ) ⊜ true
-        @test _analyze_deps_compat_2(
-            pkg,
-            root_project_path,
+            "deps",
+        )
+        @test isempty(result)
+        result = find_missing_deps_compat(
             DictSA(
                 "deps" => DictSA("PkgA" => "229717a1-0d13-4dfb-ba8f-049672e31205"),
                 "compat" => DictSA("julia" => "1", "PkgA" => "1.0"),
             ),
-        ) ⊜ true
+            "deps",
+        )
+        @test isempty(result)
         @testset "does not have `deps`" begin
-            # Not sure if it should fail or passs:
-            t = _analyze_deps_compat_2(pkg, root_project_path, DictSA())
-            @test t ⊜ true
-            @test occursin("does not have `deps`", string(t))
+            result = find_missing_deps_compat(DictSA(), "deps")
+            @test isempty(result)
         end
     end
     @testset "failure" begin
         @testset "does not have `compat`" begin
-            t = _analyze_deps_compat_2(
-                pkg,
-                root_project_path,
+            result = find_missing_deps_compat(
                 DictSA("deps" => DictSA("PkgA" => "229717a1-0d13-4dfb-ba8f-049672e31205")),
+                "deps",
             )
-            @test t ⊜ false
-            @test occursin("does not have `compat`", string(t))
+            @test length(result) == 1
+            @test [pkg.name for pkg in result] == ["PkgA"]
         end
 
         @testset "does not specify `compat` for PkgA" begin
-            t = _analyze_deps_compat_2(
-                pkg,
-                root_project_path,
+            result = find_missing_deps_compat(
                 DictSA(
                     "deps" => DictSA("PkgA" => "229717a1-0d13-4dfb-ba8f-049672e31205"),
                     "compat" => DictSA("julia" => "1"),
                 ),
+                "deps",
             )
-            @test t ⊜ false
-            @test occursin("does not specify `compat` for", string(t))
-            @test occursin("PkgA", string(t))
+            @test length(result) == 1
+            @test [pkg.name for pkg in result] == ["PkgA"]
         end
 
         @testset "does not specify `compat` for PkgB" begin
-            t = _analyze_deps_compat_2(
-                pkg,
-                root_project_path,
+            result = find_missing_deps_compat(
                 DictSA(
                     "deps" => DictSA(
                         "PkgA" => "229717a1-0d13-4dfb-ba8f-049672e31205",
                         "PkgB" => "3d97d89c-7c41-49ae-981c-14fe13cc7943",
                     ),
                     "compat" => DictSA("julia" => "1", "PkgA" => "1.0"),
                 ),
+                "deps",
             )
-            @test t ⊜ false
-            @test occursin("does not specify `compat` for", string(t))
-            @test occursin("PkgB", string(t))
+            @test length(result) == 1
+            @test [pkg.name for pkg in result] == ["PkgB"]
         end
     end
 end