Aqua.jl

[kunzaatko]

Minor tweak to Aqua.jl configuration in runtests.jl and add badge to README

• edit(Agua): Update issue link and mark as broken
• docs(README): Add Agua.jl badge

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fef31cd) 99.88% compared to head (f249247) 99.88%.
Report is 6 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #304   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files           7        7           
  Lines         871      900   +29     
=======================================
+ Hits          870      899   +29     
  Misses          1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jishnub]

Thanks, I didn't know about the broken flag. Perhaps we should also limit the compat bound of Aqua to v0.7

[kunzaatko]

I limited the compat of Aqua as you suggested to v0.7. That was a good catch. I also switched the plugin to the new format of Project.toml without the extra target. It is more organized that way I think and does not make any practical difference because the compat for julia is not prev1.2 where the change occured.

[jishnub]

Let's not change the format. Although this should work fine in principle, there are occasional subtle conflicts between the main Manifest.toml and the test/Manifest.toml files that are hard to debug, especially when developing the package. The older format doesn't have these issues.

[jishnub]

It seems, using the broken flag prints out the entire list of ambiguities when running the tests. I'm not sure if this is desired.

[kunzaatko]

I reverted it to the previous format.
I guess the idea behind the broken flag showing the output is, that it is broken only temporarily for a short time, and it works as a reminder... The decision is up to you though.

[kunzaatko]

Is there something that I should know about the new format? Is there a reason why I shouldn't be using it in my packages if they have compat julia=^1.2?

[jishnub]

I like the idea of the broken flag, but perhaps Aqua should summarise the broken tests instead of printing them all. This is ultimately an upstream decision, though

[jishnub]

Is there something that I should know about the new format? Is there a reason why I shouldn't be using it in my packages if they have compat julia=^1.2?

I can't find the link immediately, but there used to be a warning that stated that the interplay between the Manifest.toml and test/Manifest.toml is still not fully worked out (and IIRC there was a Pkg PR about the test environment inheriting the main one instead of re-resolving). In the past, I've occasionally faced issues when the main and test environments load different versions of the same package, and I've found that using TestEnv.jl makes the process of instantiating the test environment much smoother.

In practice, this may not be an issue if one doesn't dev local packages, so whether you want the new format is a personal choice to be made by the developers.

[jishnub]

Looks like Aqua expects this to be on a single line

[kunzaatko]

Now it should be fine

[kunzaatko]

In the past, I've occasionally faced issues when the main and test environments load different versions of the same package

There is a test for differences between the test environment and the main environment versions. So testing with Aqua.jl should eliminate this.

[jishnub]

The Aqua test checks that the same dependencies are listed in the [targets] section and the test/Project.toml. I was referring to the case where these two are identical, but the difference is in the manifest (e.g. which branch of a dependency is added). Anyway, this PR looks good currently, other than the issue with the ambiguities being printed.

I had closed the PR accidentally, but it's open again.

[jishnub]

For future reference, I was referring to this issue: JuliaLang/Pkg.jl#1585, but perhaps this is fixed now