Skip to content

Commit 8a6c1f1

Browse files
authored
Merge pull request #179 from nichollsh/hn/testcov
Improve test coverage reporting, and increase test coverage to 70%
2 parents d52ac1c + 6d34b2d commit 8a6c1f1

23 files changed

Lines changed: 2404 additions & 151 deletions

.github/copilot-instructions.md

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,23 @@ julia agni.jl res/config/hotdry.toml # use a specific config
2323

2424
### Run all tests
2525
```bash
26-
cd test/
27-
julia --project=.. runtests.jl
26+
julia --project=. test/runtests.jl
2827
```
2928

3029
### Run fast tests only (skips integration tests)
3130
```bash
32-
cd test/
33-
julia --project=.. runtests.jl fast
31+
julia --project=. test/runtests.jl fast
3432
```
3533

3634
### Run a single test file
3735
```bash
38-
cd test/
39-
julia --project=.. -e 'include("test_phys.jl")'
36+
julia --project=. test/runtests.jl phys
4037
```
4138

4239
### Generate coverage
4340
```bash
44-
cd test/
45-
julia --project=.. --code-coverage runtests.jl
46-
julia --project=.. ../test/get_coverage.jl
41+
julia --project=. --code-coverage test/runtests.jl
42+
julia --project=. test/get_coverage.jl
4743
```
4844

4945
## Architecture

.github/workflows/install_and_test.yml

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
version: '1.11'
3535

3636
- name: Cache Julia
37-
uses: julia-actions/cache@v2
37+
uses: julia-actions/cache@v3
3838

3939
# Setup SOCRATES
4040
- name: Get SOCRATES
@@ -44,13 +44,10 @@ jobs:
4444
path: 'SOCRATES'
4545

4646
- name: Restore SOCRATES from cache
47-
uses: actions/cache@v4
47+
uses: actions/cache@v5
4848
id: cache-socrates
4949
with:
50-
path: |
51-
SOCRATES/bin
52-
SOCRATES/sbin
53-
SOCRATES/set_rad_env
50+
path: SOCRATES/
5451
key: socrates-${{ hashFiles('SOCRATES/version') }}
5552

5653
- name: Build SOCRATES if required
@@ -63,6 +60,9 @@ jobs:
6360
./build_code
6461
cd ..
6562
63+
- name: Get FastChem
64+
run: ./src/get_fastchem.sh
65+
6666
- name: Build AGNI
6767
run: |
6868
export RAD_DIR="/home/runner/work/AGNI/AGNI/SOCRATES"
@@ -76,18 +76,18 @@ jobs:
7676
run: |
7777
export RAD_DIR="/home/runner/work/AGNI/AGNI/SOCRATES"
7878
export LD_LIBRARY_PATH=""
79-
cd test/
80-
julia --project=.. --code-coverage runtests.jl
79+
julia --project=. --code-coverage test/runtests.jl
8180
8281
- name: Get coverage
8382
run: |
8483
julia --project=. test/get_coverage.jl
8584
export total=$(cat coverage.total)
8685
echo "total=$total" >> $GITHUB_ENV
86+
cat coverage.md >> "$GITHUB_STEP_SUMMARY"
8787
8888
# Upload output dir for inspection by user
8989
- name: Upload result as artifact
90-
uses: actions/upload-artifact@v4
90+
uses: actions/upload-artifact@v7
9191
with:
9292
name: test-artifact
9393
path: |
@@ -96,7 +96,7 @@ jobs:
9696
9797
# Make badge
9898
- name: Create coverage badge
99-
uses: schneegans/dynamic-badges-action@v1.7.0
99+
uses: schneegans/dynamic-badges-action@v1.8.0
100100
with:
101101
auth: ${{ secrets.GIST_TOKEN }}
102102
gistID: e20f4fa3c7811c75d34005311fef3696

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ Below is an animated example of AGNI solving for a temperature-pressure profile,
5353
* `out/` - Model output files
5454
* `res/` - Resources (configs, thermodynamic data, etc.)
5555
* `src/` - Source code
56-
* `test/` - Tests for the code
56+
* `test/` - Tests for the code (see [Testing Documentation](https://www.h-nicholls.space/AGNI/dev/howto/testing/))
5757
* `tutorials/` - Notebooks and tutorials
5858

5959
This software is available under GPLv3. Copyright (C) 2023-2026 Harrison Nicholls.

docs/src/contributing.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@
3131
- Print statements should be made through the logger where possible.
3232
- The core package code should not contain global variables, except in the phys module.
3333

34+
## Testing
35+
36+
When contributing code changes, please:
37+
- Run the test suite to ensure your changes don't break existing functionality
38+
- Add tests for new features or bug fixes
39+
- Aim for >80% line coverage for new utility functions
40+
- Keep tests fast (avoid expensive setup/teardown when possible)
41+
42+
See the [Testing Documentation](https://www.h-nicholls.space/AGNI/dev/howto/testing/) for detailed information on:
43+
- Running tests locally
44+
- Generating coverage reports
45+
- Adding new tests
46+
- Test design principles
47+
3448
## Queries and questions
3549

3650
* Ask a question about how to use or contribute to AGNI, please use the [discussions page](https://github.com/nichollsh/AGNI/discussions) or contact [Harrison Nicholls](https://www.h-nicholls.space/) directly.

docs/src/howto/testing.md

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
# Testing in AGNI
2+
3+
This document describes the testing infrastructure and how to run tests for AGNI.
4+
5+
## Running Tests
6+
7+
### Prerequisites
8+
Before running tests, ensure that the `RAD_DIR` environment variable is set to your SOCRATES installation:
9+
10+
```bash
11+
export RAD_DIR=/path/to/SOCRATES
12+
```
13+
14+
### Run All Tests
15+
From the AGNI root directory:
16+
17+
```bash
18+
julia --project=. test/runtests.jl
19+
```
20+
21+
### Run Fast Tests Only
22+
The fast test suite excludes expensive integration tests:
23+
24+
```bash
25+
julia --project=. test/runtests.jl fast
26+
```
27+
28+
## Test Coverage
29+
30+
### Generating Coverage Reports
31+
32+
#### Run Tests with Coverage
33+
From the AGNI root directory:
34+
35+
```bash
36+
julia --project=. --code-coverage test/runtests.jl
37+
```
38+
39+
#### Process Coverage Data
40+
From the AGNI root directory:
41+
42+
```bash
43+
julia --project=. test/get_coverage.jl
44+
```
45+
46+
This generates:
47+
- `coverage.info` - LCOV format coverage data
48+
- `coverage.json` - Codecov JSON format
49+
- `coverage.total` - Single number with overall coverage percentage
50+
- `coverage.md` - Markdown formatted file with report on coverage
51+
52+
The `coverage.md` file outlines:
53+
- Overall coverage percentage
54+
- Per-file coverage breakdown with color coding (🔴 <50%, 🟡 50-80%, 🟢 >80%)
55+
- Files needing attention (<50% coverage)
56+
- Quick wins (small files with 0% coverage)
57+
- Lists of uncovered line numbers
58+
59+
### Test Design Principles
60+
61+
1. **Fast by default**: Most tests avoid expensive operations like full atmosphere allocation
62+
2. **Unit-focused**: Each test targets specific functions or small groups of related functions
63+
3. **Self-contained**: Tests don't depend on external files when possible (except SOCRATES spectral files)
64+
4. **Comprehensive**: Tests cover:
65+
- Normal operation (happy path)
66+
- Edge cases (boundary conditions, empty inputs)
67+
- Error conditions (missing files, invalid inputs)
68+
- Type stability and return value validation
69+
70+
### Example Test Pattern
71+
72+
```julia
73+
using Test
74+
using AGNI
75+
76+
@testset "module_name" begin
77+
@testset "function_name" begin
78+
# Test normal case
79+
result = AGNI.module.function(valid_input)
80+
@test result > 0
81+
@test typeof(result) == Float64
82+
83+
# Test edge case
84+
result_edge = AGNI.module.function(edge_case_input)
85+
@test isfinite(result_edge)
86+
87+
# Test error handling
88+
@test_throws ErrorException AGNI.module.function(invalid_input)
89+
end
90+
end
91+
```
92+
93+
## Adding New Tests
94+
95+
When adding new tests:
96+
97+
1. Create a new test file in `test/` following the naming convention `test_<module>.jl`
98+
2. Any `test_*.jl` file in `test/` will be picked up automatically by `test/runtests.jl`, so no manual edit to `test/runtests.jl` is needed
99+
3. Follow the existing test patterns (see above)
100+
4. Avoid expensive operations:
101+
- Don't call `allocate!`, `setup!`, or `deallocate!` unless absolutely necessary
102+
- Use minimal atmosphere configurations when needed
103+
- Keep test execution time under 5 seconds per file
104+
5. Clean up temporary files created during tests
105+
6. Test both success and failure paths
106+
7. Use descriptive test names in `@testset` blocks
107+
108+
### Coverage Goals
109+
110+
- **Target**: >80% line coverage for utility modules (`phys`, `consts`, `ocean`, `guillot`, `blake`)
111+
- **Acceptable**: >50% for core simulation modules (`atmosphere`, `energy`, `solver`)
112+
- **Document**: Any intentionally untested code (e.g., platform-specific branches)
113+
114+
## Troubleshooting the tests
115+
116+
### Tests fail with "RAD_DIR not set"
117+
Ensure `RAD_DIR` points to a valid SOCRATES installation:
118+
```bash
119+
export RAD_DIR=/path/to/SOCRATES
120+
```
121+
122+
### Tests fail with "Spectral file not found"
123+
The test suite expects spectral files in `res/spectral_files/`. Ensure AGNI's resource directory is complete.
124+
125+
### Coverage is 0% or unexpectedly low
126+
- Make sure you ran tests with `--code-coverage` flag
127+
- Run `get_coverage.jl` from the AGNI root directory (not `test/`)
128+
- Check that `.cov` files were generated in `src/` and `test/`
129+
130+
## Test Maintenance
131+
132+
- Run tests before committing changes
133+
- Update tests when modifying function signatures or behavior
134+
- Keep test execution time reasonable (fast suite <30 seconds)
135+
- Regularly review coverage reports to identify untested code paths

src/atmosphere.jl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1911,6 +1911,12 @@ module atmosphere
19111911
wl, fl = spectrum.load_from_file(atmos.star_file)
19121912
end
19131913

1914+
# Check that spectrum was loaded successfully
1915+
if (length(wl) <= 1) || (length(fl) != length(wl))
1916+
@error "Invalid length of stellar spectrum '$(atmos.star_file)'"
1917+
return false
1918+
end
1919+
19141920
# Write stellar spectrum to disk in format required by SOCRATES
19151921
spectrum.write_to_socrates_format(wl, fl, socstar) || return false
19161922

src/blake.jl

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,18 @@ module blake
1616
Only works on AMD64 Linux.
1717
1818
Arguments:
19-
- `fpath::String`: Path to the file to hash.
19+
- `fpath::String` Path to the file to hash.
20+
- `quiet::Bool=false` Suppresses warnings about missing files.
2021
2122
Returns:
2223
- `String`: The BLAKE2b hash of the file.
2324
"""
24-
function hash_file(fpath::String)::String
25+
function hash_file(fpath::String; quiet::Bool=false)::String
2526
if !Sys.islinux()
2627
return "ONLY_SUPPORTED_ON_LINUX"
2728
end
2829
if !isfile(fpath)
29-
@warn "File not found '$fpath'"
30+
quiet || @warn "File not found '$fpath'"
3031
return "FILE_NOT_FOUND:$fpath"
3132
end
3233
content = strip(read(`$exec_path $fpath`, String))
@@ -39,15 +40,16 @@ module blake
3940
Assumes that there's a `.chk` file in the same folder.
4041
4142
Arguments
42-
- `fpath::String`: Path to the file to validate.
43+
- `fpath::String` Path to the file to validate
44+
- `quiet::Bool=false` Suppresses warnings
4345
4446
Returns
45-
- `Bool`: True if the file is valid, false otherwise.
47+
- `Bool` File is valid.
4648
"""
47-
function valid_file(fpath::String)::Bool
49+
function valid_file(fpath::String; quiet::Bool=false)::Bool
4850
# Return true if unsupported
4951
if !Sys.islinux()
50-
@debug "Skipping integrity check for '$fpath'"
52+
quiet || @debug "Skipping integrity check for '$fpath'"
5153
return true
5254
end
5355

@@ -57,16 +59,16 @@ module blake
5759
# Get the expected hash
5860
cpath::String = fpath*".chk"
5961
if !isfile(cpath)
60-
@warn "File not found '$cpath'"
62+
quiet || @warn "File not found '$cpath'"
6163
return false
6264
end
6365
hash_exp = strip(read(fpath*".chk", String))
6466

6567
# Check if equal
6668
if Bool(hash_exp != hash_obs)
67-
@warn "File '$fpath' failed integrity check"
68-
@warn " obs = '$hash_obs'"
69-
@warn " exp = '$hash_exp'"
69+
quiet || @warn "File '$fpath' failed integrity check"
70+
quiet || @warn " obs = '$hash_obs'"
71+
quiet || @warn " exp = '$hash_exp'"
7072
return false
7173
end
7274
return true

src/chemistry.jl

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,9 @@ module chemistry
288288

289289
end # end condensate
290290

291+
# Re-normalise VMRs, keeping condensate VMRs fixed
291292
normalise_vmrs!(atmos, i)
293+
292294
end # end i levels
293295

294296
# Ensure that all yields are positive at this point
@@ -375,14 +377,17 @@ module chemistry
375377

376378
end # end loop over condensates
377379

380+
# Layer properties
381+
for i in 1:atmos.nlev_c
382+
normalise_vmrs!(atmos, i)
383+
end
384+
atmosphere.calc_layer_props!(atmos)
385+
378386
# Set water clouds at levels where condensation occurs
379387
if "H2O" in atmos.condensates
380388
atmosphere.set_cloud!(atmos; from_yield=true)
381389
end
382390

383-
# Layer properties
384-
atmosphere.calc_layer_props!(atmos)
385-
386391
return nothing
387392
end
388393

@@ -693,25 +698,6 @@ module chemistry
693698
end # /match
694699
end # /gas
695700

696-
# Do not renormalise mixing ratios, since this is done by fastchem
697-
# If we are missing gases then that's okay.
698-
699-
# Find where T(p) drops below fastchem_floor temperature.
700-
# Make sure that regions above that use the reasonable VMR values
701-
# i_trunc::Int = 0
702-
# for i in reverse(sort(fc_levels))
703-
# if atmos.tmp[i] < atmos.fastchem_floor
704-
# i_trunc = i
705-
# break
706-
# end
707-
# end
708-
# if i_trunc > 0
709-
# # @warn @sprintf("Temperature below FC floor, at p < %.1e Pa", atmos.p[i_trunc])
710-
# for g in atmos.gas_names
711-
# atmos.gas_vmr[g][1:i_trunc] .= atmos.gas_vmr[g][i_trunc]
712-
# end
713-
# end
714-
715701
# Also record this result in gas_cvmr dictionary
716702
for g in atmos.gas_names
717703
@. atmos.gas_cvmr[g] = atmos.gas_vmr[g]

0 commit comments

Comments
 (0)