Refactor tests to use TestItems.jl

[BKaperick]

Closes issue #141.

Hello, this is a first contribution, so I am fully receptive to any feedback you have.

I have a few comments/questions:

• @testitem blocks cannot be nested, and I don't know of any way to share set-up code between them, so in test/test_circuitzoo_purification.jl I just kept the @testset blocks, but all nested within one @testitem. If someone has a recommendation for how to do this more properly I will update this.

I am still testing it locally, so I am opening this as a draft to indicate that I am actively working on this issue. I will change its status to Open once I've validated the changes are working correctly.

[Krastanov]

Hi, Bryan! Thank you for starting this! Please check out this similar contribution made to QuantumClifford.jl -- following the same style would help me a lot in keeping things consistent across projects: QuantumSavory/QuantumClifford.jl#329

QuantumSavory probably will be a more challenging case because there are a lot of ad-hoc examples tested in weird environments (faked opengl environment in CI for some of the plotting tests for instance). It will probably need some redoing of of tagging and filtering of tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.05%. Comparing base (46cb2f3) to head (3f3bce4).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   67.18%   67.05%   -0.13%     
==========================================
  Files          41       41              
  Lines        1737     1709      -28     
==========================================
- Hits         1167     1146      -21     
+ Misses        570      563       -7     

Flag	Coverage Δ
	76.11% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[BKaperick]

The only remaining failure is the persistent_tasks failure from the Aqua tests coming from QuantumSavory.jl.

Comparing the github actions run log to a recent run on master, I notice one new warning during precompilation:

│  WARNING: Method definition traceout!(QuantumOpticsBase.Operator{BL, BR, T} where T where BR where BL, Any) in module QuantumOpticsBase at /home/runner/.julia/packages/QuantumOpticsBase/QN1iB/src/operators_dense.jl:31 overwritten in module QuantumSavory at /home/runner/work/QuantumSavory.jl/QuantumSavory.jl/src/backends/quantumoptics/quantumoptics.jl:21.
│  ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.

and one new warning during running tests:

┌ Warning: Module QuantumSavory with build ID ffffffff-ffff-ffff-0000-009cee9a6d73 is missing from the cache.
│ This may mean QuantumSavory [2de2e421-972c-4cb5-a0c3-999c85908079] does not support precompilation but is imported by a module that does.

[Krastanov]

Thanks, @BKaperick ! Some of the errors you are getting are from some invasive work happening to remove piracies that is still in progress as it spans multiple packages (e.g. #143 ). It should get merged soon and then I will be able to rerun the tests here and provide a review. Apologies for the delay!

I am running a summer workshop this week (qnumerics.org) so I might be a bit slow to answer.

[Krastanov]

Apologies for not warning you about the JuMP/GraphMatching removal -- that happened because they were causing issues with installation on non-linux machines.

[BKaperick]

Hi @Krastanov no problem at all about the GraphMatching removal; I saw the other PR.

Right now I have 37 issues identified by JET, which is more than the 33 allowed. The last run on master also failed but with only 35 issues.

Doing a diff on the JET output, it looks like these are the two new issues, but looking at src/CircuitZoo/CircuitZoo.jl#L150 I don't see why this PR would cause these new issues to arise:

> ┌  @ QuantumSavory.CircuitZoo /home/runner/work/QuantumSavory.jl/QuantumSavory.jl/src/CircuitZoo/CircuitZoo.jl:150
> │┌ indexed_iterate(I::Nothing, i::Int64) @ Base ./tuple.jl:95
> ││ no matching method found `iterate(::Nothing)`: x = iterate(I::Nothing)
> │└────────────────────
> │┌ indexed_iterate(I::Nothing, i::Int64, state::Int64) @ Base ./tuple.jl:100
> ││ no matching method found `iterate(::Nothing, ::Int64)`: x = iterate(I::Nothing, state::Int64)
> │└────────────────────

[BKaperick]

After some investigation, @Krastanov this is what I've found:

Removing Cbc dependency, and the MOI from JuMP (specifically removing these two lines and the removals of Cbc and JuMP from the Project.toml in PR #153) added the two extra JET errors to master.

Changes to src/ProtocolZoo/switches.jl in #153 added the two extra JET errors to master because some [temporarily] dead code is referencing non-imported variables from Cbc and JuMP`. This isn't really an issue, but I just commented out the dead code in this PR to remove those from the JET output.

The two additional JET issues I commented above are still mysterious to me, but caught when we re-add the line ignoring JuMP.Containers from the JET test, so I interpret that to mean it's somehow external to QuantumSavory.jl so it's okay to ignore.

[Krastanov]

Thank you! Apologies for the annoyance with this. I should be able to review and merge this before the end of the week

[Krastanov]

The documentation runner indeed has been somewhat unreliable. The other runners seem fine. I will restart the doc builder a few more times if it fails again. That issue is does not seem related to your work.

[Krastanov]

Looks great! I think there was one minor typo -- running tests for it right now and this should be merged shortly.

Could you please send me an email at [email protected] with a link to this PR and your name and email so I can have NumFOCUS send you the bounty. Next batch should be sometime in October.