Background
Surfaced during testing-coverage review of PR #197.
download_era5_day (ext/NumericalEarthCDSAPIExt.jl:162) and download_era5_multivar_day (line 347) each inline the construction of the CDS request dictionary (variable list, year/month/day/time strings, area, pressure_level). The shape is currently only validated end-to-end by hitting CDS — typos or off-by-one zero-padding errors only surface as opaque server 4xx responses.
A coverage-report inspection confirms this: at the current PR HEAD, NumericalEarthCDSAPIExt.jl is at ~62% line coverage with 90 missed lines, and the bulk of the uncovered lines sit inside these per-day download paths past the skip_existing early-return. Without a CDS-callable test, those lines are effectively unreachable from the unit suite.
Proposed refactor
Extract a pure helper:
build_era5_request(name_or_names, dataset, datetimes; region) -> Dict{String, Any}
…that both download_era5_day (line 162), download_era5_multivar_day (line 347), and the Metadatum overload (line 105) delegate to.
Tests this enables
Pure dict-shape assertions, no network:
- Single-level dataset: no
pressure_level key
- Pressure-level dataset:
pressure_level is sorted hPa strings
BoundingBox region produces an area key in [N, W, S, E] order
Column region produces an area key with the right ε
- Multiple datetimes on the same day produce a
time array, not a single string
- Zero-padded month/day/hour (
"02", not "2")
Why it's worth doing
- Bug surface. The request shape is what CDS actually consumes; today a typo there is invisible until a server 4xx, with no useful local signal.
- Coverage. This is the single biggest unit-testable chunk left in the CDS extension. Extracting and testing it should bump
NumericalEarthCDSAPIExt.jl line coverage materially — most of the currently-uncovered ~90 lines live in the per-day paths that this refactor opens up to direct test.
- Mostly mechanical. Pull the dict-build out, replace each inline construction with a call to the helper, leave behavior unchanged.
Suggested as a small follow-up PR after #197 lands.
Background
Surfaced during testing-coverage review of PR #197.
download_era5_day(ext/NumericalEarthCDSAPIExt.jl:162) anddownload_era5_multivar_day(line 347) each inline the construction of the CDS request dictionary (variable list, year/month/day/time strings,area,pressure_level). The shape is currently only validated end-to-end by hitting CDS — typos or off-by-one zero-padding errors only surface as opaque server 4xx responses.A coverage-report inspection confirms this: at the current PR HEAD,
NumericalEarthCDSAPIExt.jlis at ~62% line coverage with 90 missed lines, and the bulk of the uncovered lines sit inside these per-day download paths past theskip_existingearly-return. Without a CDS-callable test, those lines are effectively unreachable from the unit suite.Proposed refactor
Extract a pure helper:
…that both
download_era5_day(line 162),download_era5_multivar_day(line 347), and theMetadatumoverload (line 105) delegate to.Tests this enables
Pure dict-shape assertions, no network:
pressure_levelkeypressure_levelis sorted hPa stringsBoundingBoxregion produces anareakey in[N, W, S, E]orderColumnregion produces anareakey with the right εtimearray, not a single string"02", not"2")Why it's worth doing
NumericalEarthCDSAPIExt.jlline coverage materially — most of the currently-uncovered ~90 lines live in the per-day paths that this refactor opens up to direct test.Suggested as a small follow-up PR after #197 lands.