From 02a70281f474265a4db5f6cf489f9b644b92382e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20G=C3=B6ttgens?= Date: Tue, 2 Apr 2024 23:31:23 +0200 Subject: [PATCH] Make broken tests less noisy (#272) --- CHANGELOG.md | 2 ++ src/ambiguities.jl | 30 ++++++++++++++++++------------ src/exports.jl | 25 ++++++++++++++++++++++--- src/piracies.jl | 30 +++++++++++++++++++----------- src/unbound_args.jl | 31 ++++++++++++++++++++----------- 5 files changed, 81 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c9291d5..76a86aea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- When supplying `broken = true` to `test_ambiguities`, `test_undefined_exports`, `test_piracies`, or `test_unbound_args`, the output is shortened. In particular, the list of offending instances is no longer printed. To get the full output, set `broken = false`. ([#272]) - Use [Changelog.jl](https://github.com/JuliaDocs/Changelog.jl) to generate the changelog, and add it to the documentation. ([#277], [#279]) - `test_project_extras` prints failures the same on all julia versions. In particular, 1.11 and nightly are no outliers. ([#275]) @@ -250,5 +251,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#250]: https://github.com/JuliaTesting/Aqua.jl/issues/250 [#255]: https://github.com/JuliaTesting/Aqua.jl/issues/255 [#256]: https://github.com/JuliaTesting/Aqua.jl/issues/256 +[#272]: https://github.com/JuliaTesting/Aqua.jl/issues/272 [#275]: https://github.com/JuliaTesting/Aqua.jl/issues/275 [#277]: https://github.com/JuliaTesting/Aqua.jl/issues/277 diff --git a/src/ambiguities.jl b/src/ambiguities.jl index a0da6b98..d973a837 100644 --- a/src/ambiguities.jl +++ b/src/ambiguities.jl @@ -8,7 +8,7 @@ false-positives. # Keyword Arguments - `broken::Bool = false`: If true, it uses `@test_broken` instead of - `@test`. + `@test` and shortens the error message. - `color::Union{Bool, Nothing} = nothing`: Enable/disable colorful output if a `Bool`. `nothing` (default) means to inherit the setting in the current process. @@ -66,20 +66,22 @@ function reprexclude(exspecs::Vector{ExcludeSpec}) end function _test_ambiguities(packages::Vector{PkgId}; broken::Bool = false, kwargs...) - num_ambiguities, strout, strerr = _find_ambiguities(packages; kwargs...) + num_ambiguities, strout, strerr = + _find_ambiguities(packages; skipdetails = broken, kwargs...) print(stderr, strerr) print(stdout, strout) if broken - @test_broken num_ambiguities == 0 + @test_broken iszero(num_ambiguities) else - @test num_ambiguities == 0 + @test iszero(num_ambiguities) end end function _find_ambiguities( packages::Vector{PkgId}; + skipdetails::Bool = false, color::Union{Bool,Nothing} = nothing, exclude::AbstractVector = [], # Options to be passed to `Test.detect_ambiguities`: @@ -98,6 +100,7 @@ function _find_ambiguities( $packages_repr, $options_repr, $exclude_repr, + $skipdetails, ) || exit(1) """ cmd = Base.julia_cmd() @@ -186,6 +189,7 @@ function test_ambiguities_impl( packages::Vector{PkgId}, options::NamedTuple, exspecs::Vector{ExcludeSpec}, + skipdetails::Bool, ) modules = map(Base.require, packages) @debug "Testing method ambiguities" modules @@ -201,18 +205,20 @@ function test_ambiguities_impl( sort!(ambiguities, by = (ms -> (ms[1].name, ms[2].name))) if !isempty(ambiguities) - printstyled("$(length(ambiguities)) ambiguities found", color = :red) + printstyled("$(length(ambiguities)) ambiguities found.\n", color = :red) println() end - for (i, (m1, m2)) in enumerate(ambiguities) - println("Ambiguity #", i) - println(m1) - println(m2) - @static if isdefined(Base, :morespecific) - ambiguity_hint(m1, m2) + if !skipdetails + for (i, (m1, m2)) in enumerate(ambiguities) + println("Ambiguity #", i) + println(m1) + println(m2) + @static if isdefined(Base, :morespecific) + ambiguity_hint(m1, m2) + println() + end println() end - println() end return isempty(ambiguities) end diff --git a/src/exports.jl b/src/exports.jl index 138a0969..52f8cb01 100644 --- a/src/exports.jl +++ b/src/exports.jl @@ -29,12 +29,31 @@ Test that all `export`ed names in `module` actually exist. # Keyword Arguments - `broken::Bool = false`: If true, it uses `@test_broken` instead of - `@test`. + `@test` and shortens the error message. """ function test_undefined_exports(m::Module; broken::Bool = false) + exports = undefined_exports(m) if broken - @test_broken undefined_exports(m) == [] + if !isempty(exports) + printstyled( + stderr, + "$(length(exports)) undefined exports detected. To get a list, set `broken = false`.\n"; + bold = true, + color = Base.error_color(), + ) + end + @test_broken isempty(exports) else - @test undefined_exports(m) == [] + if !isempty(exports) + printstyled( + stderr, + "Undefined exports detected:\n"; + bold = true, + color = Base.error_color(), + ) + show(stderr, MIME"text/plain"(), exports) + println(stderr) + end + @test isempty(exports) end end diff --git a/src/piracies.jl b/src/piracies.jl index b5bfcfa8..97c7c90a 100644 --- a/src/piracies.jl +++ b/src/piracies.jl @@ -206,7 +206,7 @@ Test that `m` does not commit type piracies. # Keyword Arguments - `broken::Bool = false`: If true, it uses `@test_broken` instead of - `@test`. + `@test` and shortens the error message. - `skip_deprecated::Bool = true`: If true, it does not check deprecated methods. - `treat_as_own = Union{Function, Type}[]`: The types in this container are considered to be "owned" by the module `m`. This is useful for @@ -216,19 +216,27 @@ Test that `m` does not commit type piracies. """ function test_piracies(m::Module; broken::Bool = false, kwargs...) v = Piracy.hunt(m; kwargs...) - if !isempty(v) - printstyled( - stderr, - "Possible type-piracy detected:\n"; - bold = true, - color = Base.error_color(), - ) - show(stderr, MIME"text/plain"(), v) - println(stderr) - end if broken + if !isempty(v) + printstyled( + stderr, + "$(length(v)) instances of possible type-piracy detected. To get a list, set `broken = false`.\n"; + bold = true, + color = Base.error_color(), + ) + end @test_broken isempty(v) else + if !isempty(v) + printstyled( + stderr, + "Possible type-piracy detected:\n"; + bold = true, + color = Base.error_color(), + ) + show(stderr, MIME"text/plain"(), v) + println(stderr) + end @test isempty(v) end end diff --git a/src/unbound_args.jl b/src/unbound_args.jl index 2e1f195e..a6413f5e 100644 --- a/src/unbound_args.jl +++ b/src/unbound_args.jl @@ -8,23 +8,32 @@ of the method. # Keyword Arguments - `broken::Bool = false`: If true, it uses `@test_broken` instead of - `@test`. + `@test` and shortens the error message. """ function test_unbound_args(m::Module; broken::Bool = false) unbounds = detect_unbound_args_recursively(m) - if !isempty(unbounds) - printstyled( - stderr, - "Unbound type parameters detected:\n"; - bold = true, - color = Base.error_color(), - ) - show(stderr, MIME"text/plain"(), unbounds) - println(stderr) - end if broken + if !isempty(unbounds) + printstyled( + stderr, + "$(length(unbounds)) instances of unbound type parameters detected. To get a list, set `broken = false`.\n"; + bold = true, + color = Base.error_color(), + ) + end @test_broken isempty(unbounds) else + if !isempty(unbounds) + printstyled( + stderr, + "Unbound type parameters detected:\n"; + bold = true, + color = Base.error_color(), + ) + show(stderr, MIME"text/plain"(), unbounds) + println(stderr) + end + @test isempty(unbounds) end end