This repository has been archived by the owner on Mar 1, 2023. It is now read-only.
Unit tests may be leaking scope #1368
Assignees
Labels
CI
Continuous Integration
Comments
|
Just got stung by this here. |
7 tasks
|
I noticed in ForwardDiff.jl, they just put each unit test inside a module. Doing that seems like a simple solution/pattern. |
4 tasks
bors bot
added a commit
that referenced
this issue
Jan 30, 2021
1947: Add some Vars wrappers r=charleskawczynski a=charleskawczynski ### Description This PR revives some old work with adding `Vars` wrappers to `BalanceLaws`, so that the DG kernels don't have to wrap arrays with every call. The previous branch I was working on got a bit out of control, so I'm leaning towards doing this more incrementally. This PR only adds vars wrappers for: - `source` - `flux_first_order!` - `flux_second_order!` When trying to rely on dispatch between `AbstractArray` and `Vars` methods, I hit method ambiguities (due to, e.g., `source!(::AtmosAcousticLinearModel, _...) = nothing`), so my thought was that appending `_arr` to the array-based methods would be the simplest solution. I'm definitely open to other ideas. Pros/cons: - [con] Performance? (based on observations in #500) - [pro] Thins the layer in which `Vars` is used, which makes VariableTemplates closer to being replaceable - [pro] Improves readability of DG kernels I also flattened out a few doc strings to make them a bit more readable. 1960: Add --nelems for Ocean config r=valeriabarra a=valeriabarra This is a quick patch of PR #1946 to add the `--nelems` CL option also for the Ocean configuration. While at it, I also printed the same `@info` that is printed in the other configs and added a missing config to the doc summary at the top of the file. I tested this with ``` $ julia --project test/Ocean/HydrostaticBoussinesq/test_3D_spindown.jl --nelems 4,4,7 ``` and the relevant output is: ``` ┌ Info: Establishing Ocean configuration for hydrostatic_spindown │ precision = Float64 │ horiz polynomial order = 4 │ vert polynomial order = 4 │ Nˣ # elems = 4 │ Nʸ # elems = 4 │ Nᶻ # elems = 7 │ domain depth = -4.00e+02 m └ MPI ranks = 1 ``` - [x] Code follows the [style guidelines](https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/) OR N/A. - [x] Unit tests are included OR N/A. - [x] Code is exercised in an integration test OR N/A. - [x] Documentation has been added/updated OR N/A. 1962: Wrap some runtests in modules r=charleskawczynski a=charleskawczynski ### Description This PR wraps some `runtests.jl` in modules. A step towards #1368. 1973: Add some compat bounds/entries, up pkgs r=charleskawczynski a=charleskawczynski ### Description Supersedes #1949, #1948, #1945 1974: Add some VariableTemplates tests r=charleskawczynski a=charleskawczynski ### Description Reviewing CodeCov, these added tests should help cover some parts of VariableTemplates that are not currently unit-tested. 1975: Remove unused parameter in SurfaceFluxes r=charleskawczynski a=charleskawczynski ### Description Remove unused parameter in SurfaceFluxes 1976: modify solid body rotation driver r=charleskawczynski a=szy21 ### Description This PR modifies solid_body_rotation_fvm.jl: - Turn off grid stretching - Change the lat lon grids in the diagnostics Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com> Co-authored-by: Valeria Barra <valeriabarra21@gmail.com> Co-authored-by: Zhaoyi Shen <pkuszy@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Description
At one time, I found that this
AtmosModelcall was picking up thisAtmosModelreference. Maybe we should implement something like SafeTestsets.jl or create custom test sets to avoid leaking scope in our unit tests.Additional context
Add any other reasons why this should be addressed.
For CLIMA Developers
The text was updated successfully, but these errors were encountered: