Skip to content

Commit

Permalink
Make broken tests less noisy (#272)
Browse files Browse the repository at this point in the history
  • Loading branch information
lgoettgens authored Apr 2, 2024
1 parent 6f3067c commit 02a7028
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 37 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])

Expand Down Expand Up @@ -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
30 changes: 18 additions & 12 deletions src/ambiguities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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`:
Expand All @@ -98,6 +100,7 @@ function _find_ambiguities(
$packages_repr,
$options_repr,
$exclude_repr,
$skipdetails,
) || exit(1)
"""
cmd = Base.julia_cmd()
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
25 changes: 22 additions & 3 deletions src/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
30 changes: 19 additions & 11 deletions src/piracies.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
31 changes: 20 additions & 11 deletions src/unbound_args.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 02a7028

Please sign in to comment.