add Test API for better Aqua integration#143
Conversation
|
ok the code here is a bit manual, but I got what I hope are decently legible errors out: ❯ julia --project -e 'using ExplicitImports, Test; test_explicit_imports(ExplicitImports)'
No implicit imports: Test Failed at /Users/eph/ExplicitImports/ext/TestExt.jl:47
Expression: isempty(missing_explicit_imports)
Evaluated: isempty(["using Test: Test # at /Users/eph/ExplicitImports/ext/TestExt.jl:4:7", "using Test: @test # at /Users/eph/ExplicitImports/ext/TestExt.jl:34:22", "using Test: @testset # at /Users/eph/ExplicitImports/ext/TestExt.jl:27:6"])
Stacktrace:
[1] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:680 [inlined]
[2] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:47 [inlined]
[3] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[4] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:30 [inlined]
[5] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[6] _test_explicit_imports(package::Module, file::String; no_implicit_imports::Bool, no_stale_explicit_imports::Bool, all_explicit_imports_via_owners::Bool, all_explicit_imports_are_public::Bool, all_qualified_accesses_via_owners::Bool, all_qualified_accesses_are_public::Bool, no_self_qualified_accesses::Bool)
@ TestExt ~/ExplicitImports/ext/TestExt.jl:28
No stale explicit imports: Test Failed at /Users/eph/ExplicitImports/ext/TestExt.jl:73
Expression: isempty(stale_explicit_imports)
Evaluated: isempty(["unused explicit import in ExplicitImports: parse # imported at /Users/eph/ExplicitImports/src/ExplicitImports.jl:9:30"])
Stacktrace:
[1] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:680 [inlined]
[2] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:73 [inlined]
[3] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[4] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:55 [inlined]
[5] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[6] _test_explicit_imports(package::Module, file::String; no_implicit_imports::Bool, no_stale_explicit_imports::Bool, all_explicit_imports_via_owners::Bool, all_explicit_imports_are_public::Bool, all_qualified_accesses_via_owners::Bool, all_qualified_accesses_are_public::Bool, no_self_qualified_accesses::Bool)
@ TestExt ~/ExplicitImports/ext/TestExt.jl:28
Explicit imports are public: Test Failed at /Users/eph/ExplicitImports/ext/TestExt.jl:111
Expression: isempty(non_public_explicit_imports)
Evaluated: isempty(["using ExplicitImports: check_file # not public, at /Users/eph/ExplicitImports/ext/TestExt.jl:3:24", "using ExplicitImports: choose_exporter # not public, at /Users/eph/ExplicitImports/ext/TestExt.jl:3:36"])
Stacktrace:
[1] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:680 [inlined]
[2] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:111 [inlined]
[3] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[4] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:100 [inlined]
[5] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[6] _test_explicit_imports(package::Module, file::String; no_implicit_imports::Bool, no_stale_explicit_imports::Bool, all_explicit_imports_via_owners::Bool, all_explicit_imports_are_public::Bool, all_qualified_accesses_via_owners::Bool, all_qualified_accesses_are_public::Bool, no_self_qualified_accesses::Bool)
@ TestExt ~/ExplicitImports/ext/TestExt.jl:28
Qualified accesses are public: Test Failed at /Users/eph/ExplicitImports/ext/TestExt.jl:149
Expression: isempty(non_public_qualified_accesses)
Evaluated: isempty(["ExplicitImports._test_explicit_imports # not public, at /Users/eph/ExplicitImports/ext/TestExt.jl:17:26"])
Stacktrace:
[1] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:680 [inlined]
[2] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:149 [inlined]
[3] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[4] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:138 [inlined]
[5] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[6] _test_explicit_imports(package::Module, file::String; no_implicit_imports::Bool, no_stale_explicit_imports::Bool, all_explicit_imports_via_owners::Bool, all_explicit_imports_are_public::Bool, all_qualified_accesses_via_owners::Bool, all_qualified_accesses_are_public::Bool, no_self_qualified_accesses::Bool)
@ TestExt ~/ExplicitImports/ext/TestExt.jl:28
No self qualified accesses: Test Failed at /Users/eph/ExplicitImports/ext/TestExt.jl:168
Expression: isempty(self_qualified_accesses)
Evaluated: isempty(["ExplicitImports.parent # at /Users/eph/ExplicitImports/src/deprecated.jl:79:21"])
Stacktrace:
[1] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:680 [inlined]
[2] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:168 [inlined]
[3] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[4] macro expansion
@ ~/ExplicitImports/ext/TestExt.jl:157 [inlined]
[5] macro expansion
@ ~/.julia/juliaup/julia-1.12.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.12/Test/src/Test.jl:1776 [inlined]
[6] _test_explicit_imports(package::Module, file::String; no_implicit_imports::Bool, no_stale_explicit_imports::Bool, all_explicit_imports_via_owners::Bool, all_explicit_imports_are_public::Bool, all_qualified_accesses_via_owners::Bool, all_qualified_accesses_are_public::Bool, no_self_qualified_accesses::Bool)
@ TestExt ~/ExplicitImports/ext/TestExt.jl:28
Test Summary: | Pass Fail Total Time
ExplicitImports | 2 5 7 1.9s
No implicit imports | 1 1 0.9s
No stale explicit imports | 1 1 0.2s
Explicit imports via owners | 1 1 0.2s
Explicit imports are public | 1 1 0.2s
Qualified accesses via owners | 1 1 0.1s
Qualified accesses are public | 1 1 0.1s
No self qualified accesses | 1 1 0.1s
RNG of the outermost testset: Random.Xoshiro(0x6f5a2ecae3c3f0c4, 0x215566042c2e427f, 0x846a6df85c32222a, 0x28b62f25432235a0, 0x8669d0b60b38da73) |
|
I decided to make it not an extension, since Test is a super light dep, this is pretty useful/core functionality, and I didn't like the stub thing (nor did I want to make 6 more stubs for each individual function). @gdalle can you let me know if this API works for Aqua's needs? |
|
I added a small optimization so the Test Summary: | Pass Fail Total Time
ExplicitImports | 3 4 7 7.7s
No implicit imports | 1 1 5.6s
No stale explicit imports | 1 1 0.3s
Explicit imports via owners | 1 1 0.3s
Explicit imports are public | 1 1 0.3s
Qualified accesses via owners | 1 1 0.3s
Qualified accesses are public | 1 1 0.3s
No self qualified accesses | 1 1 0.4sand here is without: Test Summary: | Pass Fail Total Time
ExplicitImports | 3 4 7 42.2s
No implicit imports | 1 1 5.8s
No stale explicit imports | 1 1 5.6s
Explicit imports via owners | 1 1 5.6s
Explicit imports are public | 1 1 5.7s
Qualified accesses via owners | 1 1 5.7s
Qualified accesses are public | 1 1 6.6s
No self qualified accesses | 1 1 7.2sSince we don't have a mega-check* function, the latter best represents the status quo, no analysis reuse between checks. |
There was a problem hiding this comment.
Pull request overview
This PR adds Test.jl-style wrappers around ExplicitImports check functions to enable better integration with Aqua-style testing workflows. The new test_* functions provide user-friendly test failures with detailed location information instead of throwing exceptions.
Changes:
- Added new
test_explicit_imports.jlfile with Test.jl wrappers for all check functions - Modified all
check_*functions to supportthrow=falseparameter for returning exceptions instead of throwing - Added
file_analysisparameter threading to enable performance optimization by reusing analysis across multiple checks - Updated documentation and added Test/MetaTesting dependencies
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test_explicit_imports.jl | New file with Test.jl wrapper functions (test_*) and main test_explicit_imports aggregator function |
| src/checks.jl | Added throw=false and file_analysis parameters to all check functions to support test wrappers |
| src/improper_explicit_imports.jl | Added file_analysis parameter for performance optimization |
| src/improper_qualified_accesses.jl | Added file_analysis parameter for performance optimization |
| src/ExplicitImports.jl | Added Test imports and exported new test_* functions |
| test/runtests.jl | Added tests for new test_* functions and MetaTesting import |
| docs/src/api.md | Added documentation for new test API |
| README.md | Added description of test_* functions |
| Project.toml | Added Test and MetaTesting dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw || return ex | ||
| Base.throw(ex) |
There was a problem hiding this comment.
The throw pattern is incorrect. When throw is true, the expression throw || return ex evaluates to true and short-circuits, never reaching Base.throw(ex). When throw is false, it returns early. The correct pattern should be !throw && return ex followed by Base.throw(ex), or simply throw ? Base.throw(ex) : return ex.
There was a problem hiding this comment.
not true, but I do think throw ? Base.throw(ex) : return ex is more clear...
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…es (#165) * expand `ignore` functionality to consistently allow ignoring submodules of the package * validate ignores & tweak wording * add tests for validation * tweak wording * add top-level ignore to `test_` fn * rm unnecessary fn * rm another unused fn
I started working on this 6 months ago, figured I'd put up the PR as I'm thinking about Aqua again
update: used codex-5.2 to rewrite this in a more verbose way that gives better errors