Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
2 changes: 2 additions & 0 deletions .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ jobs:
version: 1
- uses: julia-actions/cache@v2
- uses: julia-actions/julia-docdeploy@v1
with:
install-package: false
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
DOCUMENTER_KEY: ${{ secrets.SSH_KEY }}
8 changes: 1 addition & 7 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,15 @@ jobs:
strategy:
fail-fast: false
matrix:
os: ["ubuntu-latest"]
os: ['ubuntu-latest']
Comment thread
gdalle marked this conversation as resolved.
Outdated
julia-version:
- '1.12-nightly'
- '1.11'
- '1.10'
- '1.9'
- '1.8'
- '1.7'
- '1.6'
- 'nightly'
include:
- os: windows-latest
julia-version: '1'
- os: windows-latest
julia-version: '1.6'
- os: windows-latest
julia-version: '1.12-nightly'
- os: windows-latest
Expand Down
29 changes: 4 additions & 25 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

- The minimum supported julia version is increased to 1.6. ([#328])
- Explicit imports are tested using ExplicitImports.jl ([#369])

## Version [v0.8.14] - 2025-08-04

Expand Down Expand Up @@ -41,33 +42,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- No longer call `@testset` for testsets that are skipped. ([#319])


## Version [v0.8.9] - 2024-10-15

### Changed

- Change `test_ambiguities` to only return ambiguities that happen in the target package. ([#309])


## Version [v0.8.8] - 2024-10-10

### Changed

- Improved the documentation of `test_persisten_tasks`. ([#297])


## Version [v0.8.7] - 2024-04-09

- Reverted [#285], which was originally released in [v0.8.6], but caused a regression. ([#287], [#288])


## Version [v0.8.6] - 2024-04-09

### Changed

- The output of `test_ambiguities` now gets printed to stderr instead of stdout. ([#281])


## Version [v0.8.5] - 2024-04-03

### Changed
Expand All @@ -76,45 +72,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- 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])


## Version [v0.8.4] - 2023-12-01

### Added

- `test_persistent_tasks` now accepts an optional `expr` to run in the precompile package. ([#255])
+ The `expr` option lets you test whether your precompile script leaves any dangling Tasks
- The `expr` option lets you test whether your precompile script leaves any dangling Tasks
Comment thread
gdalle marked this conversation as resolved.
Outdated
or Timers, which would make it unsafe to use as a dependency for downstream packages.


## Version [v0.8.3] - 2023-11-29

### Changed

- `test_persistent_tasks` is now less noisy. ([#256])
- Completely overhauled the documentation. Every test now has its dedicated page. ([#250])


## Version [v0.8.2] - 2023-11-16

### Changed

- `test_persistent_tasks` no longer clears the environment of the subtask. Instead, it modifies `LOAD_PATH` directly to make stdlibs work. ([#241])


## Version [v0.8.1] - 2023-11-16

### Changed

- `test_persistent_tasks` now redirects stdout and stderr of the created subtask. Furthermore, the environment of the subtask gets cleared to allow default values for `JULIA_LOAD_PATH` to work. ([#240])


## Version [v0.8.0] - 2023-11-15

### Added

- Two additions check whether packages might block precompilation on Julia 1.10 or higher: ([#174])
+ `test_persistent_tasks` tests whether "your" package can safely be used as a dependency for downstream packages. This test is enabled for the default testsuite `test_all`, but you can opt-out by supplying `persistent_tasks=false` to `test_all`. [BREAKING]
+ `find_persistent_tasks_deps` is useful if "your" package hangs upon precompilation: it runs `test_persistent_tasks` on all the things you depend on, and may help isolate the culprit(s).
- `test_persistent_tasks` tests whether "your" package can safely be used as a dependency for downstream packages. This test is enabled for the default testsuite `test_all`, but you can opt-out by supplying `persistent_tasks=false` to `test_all`. [BREAKING]
- `find_persistent_tasks_deps` is useful if "your" package hangs upon precompilation: it runs `test_persistent_tasks` on all the things you depend on, and may help isolate the culprit(s).

### Changed

Expand All @@ -130,7 +121,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `test_project_toml_formatting` has been removed. Thus, the kwarg `project_toml_formatting` to `test_all` no longer exists. ([#209]) [BREAKING]


## Version [v0.7.4] - 2023-10-24

### Added
Expand All @@ -142,7 +132,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- The docstring for `test_stale_deps` explains the situation with package extensions. ([#203])
- The logo of Aqua.jl has been updated. ([#128])


## Version [v0.7.3] - 2023-09-25

### Added
Expand All @@ -153,21 +142,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `test_piracy` no longer prints warnings for methods where the third argument is a `TypeVar`. ([#188])


## Version [v0.7.2] - 2023-09-19

### Changed

- `test_undefined_exports` additionally prints the modules of the undefined exports in the failure report. ([#177])


Comment thread
gdalle marked this conversation as resolved.
## Version [v0.7.1] - 2023-09-05

### Fixed

- `test_piracy` no longer reports type piracy in the kwsorter, i.e. `kwcall` should no longer appear in the report. ([#171])


## Version [v0.7.0] - 2023-08-29

### Added
Expand All @@ -184,7 +170,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `test_ambiguities` prints less unnecessary whitespace. ([#158])
- `test_ambiguities` no longer hangs indefinitely when there are many ambiguities. ([#166])


## Version [v0.6.7] - 2023-09-19

### Changed
Expand All @@ -196,21 +181,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `test_ambiguities` prints less unnecessary whitespace. ([#158])
- Fix `test_piracy` for some methods with arguments of custom subtypes of `Function`. ([#170])


## Version [v0.6.6] - 2023-08-24

### Fixed

- `test_ambiguities` no longer hangs indefinitely when there are many ambiguities. ([#166])


## Version [v0.6.5] - 2023-06-26

### Fixed

- Typo when calling kwargs. ([#153])


## Version [v0.6.4] - 2023-06-25

### Added
Expand All @@ -225,7 +207,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Callable objects with type parameters no longer error in `test_ambiguities`' kwarg `exclude`. ([#142])


## Version [v0.6.3] - 2023-06-05

### Changed
Expand All @@ -237,7 +218,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `test_deps_compat`'s kwarg `ignore` now works as intended. ([#130])
- Weakdeps are not reported as stale by `test_stale_deps` anymore. ([#135])


## Version [v0.6.2] - 2023-06-02

### Added
Expand All @@ -255,7 +235,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Clarified the error message of `test_unbound_args`. ([#103])
- Clarified the error message of `test_project_toml_formatting`. ([#122])


<!-- Links generated by Changelog.jl -->

[v0.6.2]: https://github.com/JuliaTesting/Aqua.jl/releases/tag/v0.6.2
Expand Down
6 changes: 5 additions & 1 deletion Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ authors = ["Takafumi Arakaki <aka.tkf@gmail.com> and contributors"]
version = "1.0.0-DEV"

[deps]
ExplicitImports = "7d51a73a-1435-4ff3-83d9-f097790105c7"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[compat]
ExplicitImports = "1.15.0"
Pkg = "1.6"
TOML = "1.0.3"
Test = "<0.0.1, 1"
julia = "1.6"
julia = "1.10"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
Expand Down
6 changes: 6 additions & 0 deletions docs/Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
[deps]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
Changelog = "5217a498-cd5d-4ec6-b8c2-9b85a09b6e3e"
Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
DocumenterInterLinks = "d12716ef-a0f6-4df4-a9f1-a5a34e75c656"

[compat]
Changelog = "1"
Documenter = "1"
DocumenterInterLinks = "1.1.0"

[sources]
Aqua = { path = ".." }
7 changes: 7 additions & 0 deletions docs/make.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Documenter, Aqua, Changelog
using DocumenterInterLinks: InterLinks

# Generate a Documenter-friendly changelog from CHANGELOG.md
Changelog.generate(
Expand All @@ -8,6 +9,10 @@ Changelog.generate(
repo = "JuliaTesting/Aqua.jl",
)

links = InterLinks(
"ExplicitImports" => "https://juliatesting.github.io/ExplicitImports.jl/stable/",
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this needed for / what does it gain us?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It allows for a direct link to the docstring of ExplicitImports.test_explicit_imports, so that we don't have to separately maintain a list of its keyword arguments

makedocs(;
sitename = "Aqua.jl",
format = Documenter.HTML(;
Expand All @@ -30,9 +35,11 @@ makedocs(;
"piracies.md",
"persistent_tasks.md",
"undocumented_names.md",
"explicit_imports.md",
],
"release-notes.md",
],
plugins = [links],
)

deploydocs(; repo = "github.com/JuliaTesting/Aqua.jl", push_preview = true)
7 changes: 7 additions & 0 deletions docs/src/explicit_imports.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Explicit imports

## [Test function](@id test_explicit_imports)

```@docs
Aqua.test_explicit_imports
```
18 changes: 15 additions & 3 deletions src/Aqua.jl
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
module Aqua

using Base: Docs, PkgId, UUID
using Pkg: Pkg, TOML, PackageSpec
using Pkg.Types: VersionSpec, semver_spec
using Test
using Pkg: Pkg, PackageSpec
using Pkg.Types: VersionSpec
using Pkg.Versions: semver_spec
using Test: Test, @test, @test_broken, @testset, detect_ambiguities
using TOML: TOML

using ExplicitImports: ExplicitImports

include("utils.jl")
include("ambiguities.jl")
Expand All @@ -16,6 +19,7 @@ include("deps_compat.jl")
include("piracies.jl")
include("persistent_tasks.jl")
include("undocumented_names.jl")
include("explicit_imports.jl")

"""
test_all(testtarget::Module)
Expand All @@ -31,6 +35,7 @@ Run the following tests on the module `testtarget`:
* [`test_piracies(testtarget)`](@ref test_piracies)
* [`test_persistent_tasks(testtarget)`](@ref test_persistent_tasks)
* [`test_undocumented_names(testtarget)`](@ref test_undocumented_names)
* [`test_explicit_imports(testtarget)`](@ref test_explicit_imports)

The keyword argument `\$x` (e.g., `ambiguities`) can be used to
control whether or not to run `test_\$x` (e.g., `test_ambiguities`).
Expand All @@ -47,6 +52,7 @@ passed to `\$x` to specify the keyword arguments for `test_\$x`.
- `piracies = true`
- `persistent_tasks = true`
- `undocumented_names = false`
- `explicit_imports = true`
"""
function test_all(
testtarget::Module;
Expand All @@ -59,6 +65,7 @@ function test_all(
piracies = true,
persistent_tasks = true,
undocumented_names = false,
explicit_imports = false,
)
if ambiguities !== false
@testset "Method ambiguity" begin
Expand Down Expand Up @@ -108,6 +115,11 @@ function test_all(
test_undocumented_names(testtarget; askwargs(undocumented_names)...)
end
end
if explicit_imports !== false
@testset "Explicit imports" begin
test_explicit_imports(testtarget; askwargs(explicit_imports)...)
end
end
end

end # module
13 changes: 13 additions & 0 deletions src/explicit_imports.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
test_explicit_imports(m::Module; kwargs...)

Run the various tests provided by the package [ExplicitImports.jl](https://github.com/ericphanson/ExplicitImports.jl).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand: if ExplicitImports.test_explicit_imports already exists, why do we need to add this to Aqua at all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To have everything in one place instead of requiring users to combine several packages when testing code quality

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But we are missing JET report_package, SnoopeCompile invalidation tests, Runic (or JuliaFormatter???) code formatting.

Sarcasm aside, I don't agree with this being a useful goal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You think this is sarcasm but I actually think these would be other legitimate additions to Aqua

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main question is what the community wants, because you and I are obviously on opposite sides ^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The main question is what the community wants

Is it, though? It sure is useful what "the community" wants, but it is also exceedingly difficult to actually find that out.

In particular don't find 15 likes on an issue convincing: first off, those are not really high numbers (relative to the size of the Julia community). Moreover you don't see dislikes / don't care / "am afraid of this". And finally I am not convinced that this actually means something (it is not uncommon for people to "like" something in the abstract but then not "like" it in practice once it is there)

But most importantly to me, the set of people who even see these discussions, and comments/likes, is highly self-selected: People who see it are likely to already be interested in enforcing standards, of dealing with the resulting work, they might even read the release notes for Aqua and opt-in to new checks. (Plus: one can only "like" in Discourse, there is no way to "dislikes"; and 7 or 15 are also not very impressive numbers to start with)

But in my work and daily interactions with people who need to work with Julia code and packages but just want to get stuff done, I see a very different picture. There Julia and package updates are seen as something to dread, as something that causes constant churn by introducing breaking changes. I am glad when I can convince these people to use Aqua at all. Each new test is a new hurdle that needs to be carefully weighed: how important is this test, how easy is it to understand what is even about (and why it matters) how hard is it to make it pass (other than turning it off)?

In my subjective view, adding ExplicitImports here is a step to far. No offense intended: I think it's wonderful this package exists, and I am sure it is beneficial for many people. I also think we should mention it (and some other tools and "best practices", such as JET) in the README and manual.

And JET, SnoopCompile etc. are all nice tools in theory, but like the "problems" this PR "revealed" in Aqua itself, they often are very tricky to resolve and require convincing other package authors and even Julia itself to make changes.

Some people love spending their time with that.

Others need to get their job done first.

Again, this is a tricky balance, and admittedly where the cutoff is is subjective. For me one rule of thumb is that stuff were a fix often requires changes outside of your own package is too far.

There is also the classic dependency problem: by tying Aqua to ExplicitImports we also become beholden to decisions you make: if ExplicitImports decides to make a minor release with a stricter check, then every Aqua user is exposed to that, and Aqua will receive the issues complaining about that.

If you really want a "one stop solution"), I suggest to make a new HardcoreAqua package (well, with a better name), which uses Aqua and ExplicitImports, but and if you like alsos enforce isempty(JET.get_reports(JET.report_package(YourPackage))) and run some SnoopCompile checks; and runs Runic to verify code formatting, and whatever you like.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think Aqua has a pretty general scope:

Auto QUality Assurance for Julia packages

There's nothing that actually unifies the particular set of checks it does under one theme, they are just a bunch of useful checks. So to me it seems weird to say whatever checks already exist are OK, but other checks are not. And test_all is inherently a one-stop-shop thing, it's run all the checks you can.

Here's a random idea:

  • deprecate test_all
  • introduce test_all_v1 which maps to the existing set of checks and will never get new checks nor get deprecated
  • introduce test_all_v2 which adds ExplicitImports' checks
  • future check additions do not need to be a breaking release of Aqua, they are instead a new function

So anyone can easily stick with test_all_v1 forever, but also can keep up with compat and bugfixes (don't need to be stuck on an old version of Aqua). Then if they want to opt-in to more checks, they can go to v2.

Could also have Aqua.latest_check_version() = 2 so users can get test failures if they aren't on the latest version, if they want, or introduce test_all_latest.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of "versions" we could call these "levels of strictness". Like e.g. test_all(;strictnesslevel=1). Or make a type out of it: test_all(StrictnessLevel(1)) which then allows to have test_all(StrictnessLevelHighest()). Or what ever else makes s good API.


# Keyword Arguments

Same as those of the function [`ExplicitImports.test_explicit_imports`](@extref).
"""
function test_explicit_imports(m::Module; kwargs...)
# TODO: explicitly list kwargs here?
ExplicitImports.test_explicit_imports(m; kwargs...)
end
Loading
Loading