Issue with Running Tests: Need to Add PiCLES Manually

[Sumanshekhar17]

Hi @mochell, While following the quick start guide, I encountered an issue when trying to run the T04_2D_reg_test.jl file. Specifically, I had to manually add the PiCLES package in the test repository in order to run the test file, which can be confusing with current documentation.

Steps to Reproduce:

1. Clone the PiCLES repository as described in the quick start guide.
2. Navigate to the PiCLES directory.
3. Activate the environment and install dependencies using:

using Pkg
Pkg.activate(".")
Pkg.instantiate()

Navigate to the tests directory.

Attempt to run the T04_2D_reg_test.jl file using:

include("T04_2D_reg_test.jl")

  Activating project at `~/Desktop/PiCLES/PiCLES/tests/PiCLES`
ERROR: LoadError: ArgumentError: Package PiCLES not found in current path.
- Run `import Pkg; Pkg.add("PiCLES")` to install the PiCLES package.
Stacktrace:
 [1] macro expansion
   @ ./loading.jl:1772 [inlined]
 [2] macro expansion
   @ ./lock.jl:267 [inlined]
 [3] __require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1753
 [4] #invoke_in_world#3
   @ ./essentials.jl:926 [inlined]
 [5] invoke_in_world
   @ ./essentials.jl:923 [inlined]
 [6] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:1746
 [7] include(fname::String)
   @ Base.MainInclude ./client.jl:489
 [8] top-level scope
   @ REPL[2]:1
in expression starting at /Users/ss4338/Desktop/PiCLES/PiCLES/tests/T04_2D_reg_test.jl:9

Workaround
I resolved this by manually adding the PiCLES package while I was in the test repository using:

julia> Pkg.activate(".")
  Activating new project at `~/Desktop/PiCLES/PiCLES/tests`

and then run

include("T04_2D_reg_test.jl")

Additionally, the test case (T04_2D_reg_test.jl) requires the Oceananigans package. This package seems to be missing from the environment, possibly because it's not listed in the Project.toml or Manifest.toml files. Adding Oceananigans manually using Pkg.add("Oceananigans") should resolve the issue.

Can we add a paragraph on why it needs Oceananigans.jl ?

[glwagner]

You may be interested this tool: https://github.com/JuliaTesting/TestEnv.jl

Not sure it applies here, but test environments can have requirements that are additional to those required strictly by the package.

[Sumanshekhar17]

Thanks @glwagner .

Also @mochell in the current version, T04_2D_reg_test.jl is missing the import of Revise package. Maybe you want the user to include Revise in the Julia session and that's why not included in your code?

using Revise

Without this the code isn't running!

Here is the error message that I was getting -

julia> include("T04_2D_reg_test.jl")
  Activating project at `~/Desktop/PiCLES/PiCLES/tests/PiCLES`
ERROR: LoadError: UndefVarError: `Revise` not defined
Stacktrace:
 [1] top-level scope
   @ ~/Desktop/PiCLES/PiCLES/tests/T04_2D_reg_test.jl:72
 [2] include(fname::String)
   @ Base.MainInclude ./client.jl:489
 [3] top-level scope
   @ REPL[11]:1
in expression starting at /Users/ss4338/Desktop/PiCLES/PiCLES/tests/T04_2D_reg_test.jl:72

and it was referring to line 72, here is the code where Revise is being used -

# define ODE system and parameters
particle_system = PW.particle_equations(u, v, γ=Const_ID.γ, q=Const_ID.q);

default_ODE_parameters = (r_g=0.85, C_α=Const_Scg.C_alpha,
    C_φ=Const_ID.c_β, C_e=Const_ID.C_e, g=9.81)

Revise.retry()

[glwagner]

Why would Revise be used in a test? @Sumanshekhar17 can you please link to the code / lines in the code in your comments, its hard to follow along without them.

[Sumanshekhar17]

Sorry I updated my comment. I don't know why Revise is being used but after including this package the code ran fine!
code snippet from T04_2D_reg_test.jl where I added one line to include this Revise

#using Plots

using Pkg
Pkg.activate("PiCLES/")

import Plots as plt
using Setfield, IfElse
using Revise
using PiCLES.ParticleSystems: particle_waves_v5 as PW

[glwagner]

Can you paste a hyperlink to the code into this thread?

[glwagner]

Something like this: https://github.com/mochell/PiCLES/blob/2f0c987f7871b3a59d8ba2eae1b7b02ee6a65d07/src/PiCLES.jl#L4

I just don't know where in the repo the code is you're referring to. It's also helpful to link to individual lines!

[Sumanshekhar17]

Oh yeah, here is the code -

https://github.com/mochell/PiCLES/blob/2f0c987f7871b3a59d8ba2eae1b7b02ee6a65d07/tests/T04_2D_reg_test.jl#L72

[glwagner]

Nice thanks. Note when you put a hyperlink to code, if you do not wrap it in text, it will display the code inline which is very helpful for reading a comment quickly. (I have edited your comment to use this feature.) It is also possible to link to a range of lines, eg

https://github.com/mochell/PiCLES/blob/2f0c987f7871b3a59d8ba2eae1b7b02ee6a65d07/tests/T04_2D_reg_test.jl#L60-L76

do that you edit the hyperlink. To see the syntax, try to "edit" my post, and you will see how I added a line range to the end of the hyperlink.

[glwagner]

As for

https://github.com/mochell/PiCLES/blob/2f0c987f7871b3a59d8ba2eae1b7b02ee6a65d07/tests/T04_2D_reg_test.jl#L72

presumably its irrelevant and can be deleted.

I think there is a broader problem with the tests. In the first place the correct directory name is test --- not tests:

https://github.com/mochell/PiCLES/tree/main/tests

This matters because the tests must be runnable via

using Pkg
Pkg.test()

from the package environment. This is required for CI. This comes from the docstring for Pkg.test:

help?> Pkg.test
  Pkg.test(; kwargs...)
  Pkg.test(pkg::Union{String, Vector{String}; kwargs...)
  Pkg.test(pkgs::Union{PackageSpec, Vector{PackageSpec}}; kwargs...)

  Keyword arguments:

    •  coverage::Bool=false: enable or disable generation of coverage statistics.

    •  allow_reresolve::Bool=true: allow Pkg to reresolve the package versions in the test environment

    •  julia_args::Union{Cmd, Vector{String}}: options to be passed the test process.

    •  test_args::Union{Cmd, Vector{String}}: test arguments (ARGS) available in the test process.

  │ Julia 1.9
  │
  │  allow_reresolve requires at least Julia 1.9.

  Run the tests for package pkg, or for the current project (which thus needs to be a package) if no positional argument is given to Pkg.test. A package
  is tested by running its test/runtests.jl file.

Moreover the tests cannot make any plots nor rely on GLMakie. GLMakie will fail to load on a headless system, like those used for CI:

https://github.com/mochell/PiCLES/blob/2f0c987f7871b3a59d8ba2eae1b7b02ee6a65d07/tests/T04_2D_reg_test.jl#L26

I strongly suggest distinguishing between "examples" or "validation cases", and tests, which have a specific meaning in the context of CI.

I also would suggest that before generating documentation about how to run the this cases (some of which perhaps are meant to become tests), that this infrastructure be created. Without CI and testing the package will be difficult to develop I think because many things will break routinely.

[mochell]

yes the Revise.retry() is not relevant here, its useful for debugging.

I know @glwagner the tests are on the way, just no time to develop that all the way through.
Look here:
https://github.com/mochell/PiCLES/blob/27-write-unit-tests/test/runtests.jl

So you would exclude validations, i.e. testing if the numbers are right, from the strict tests?

[mochell]

Hi @mochell, While following the quick start guide, I encountered an issue when trying to run the T04_2D_reg_test.jl file. Specifically, I had to manually add the PiCLES package in the test repository in order to run the test file, which can be confusing with current documentation.

Workaround I resolved this by manually adding the PiCLES package while I was in the test repository using:

julia> Pkg.activate(".")
  Activating new project at `~/Desktop/PiCLES/PiCLES/tests`

and then run

include("T04_2D_reg_test.jl")

Additionally, the test case (T04_2D_reg_test.jl) requires the Oceananigans package. This package seems to be missing from the environment, possibly because it's not listed in the Project.toml or Manifest.toml files. Adding Oceananigans manually using Pkg.add("Oceananigans") should resolve the issue.

Can we add a paragraph on why it needs Oceananigans.jl ?

thanks for trying this.

• Yes, we should add Oceananigans.jl then.
• Running is rn is so clunky because it is not a registered package yet. Once I did that, the Project.toml should be easier to maintain.

[glwagner]

It doesn't need to be a registered package to make the tests, nor to add CI. Registering the package is purely a convenience for installation.

[mochell]

I revised the introduction in the branch main-initital_paper. It should work now. The repository will be public soon.