Run the tests in R CMD check, and test that the docs keep up - #26
Merged
Conversation
Two things, both about the documentation staying true rather than the
code being correct.
tests/testthat.R had test_check("datamatch") commented out. It was
commented on 23 July in a commit titled "update readme", so almost
certainly a debugging leftover that was never restored. Since then
R CMD check has reported "checking tests ... OK" while running nothing,
and CI has done the same. The suite has been passing throughout - every
report of that came from testthat::test_local(), which does run it - but
nothing was enforcing it. Uncommented: 759 tests now run under check.
The new tests guard the failure mode that keeps recurring here: code
lands, the documentation describing it does not, and nothing notices.
Seven exported functions were once implemented, tested, and absent from
the README entirely. So:
- every exported function is named in the README
- every climate index, catalog variable, and bathymetry layer is too
- DESCRIPTION does not still describe matchData() as species-specific,
and does mention what the package grew
- NEWS.md has a version heading R can parse, which is what stops the
"No news entries found" NOTE coming back
The first of those failed on its first run and found six: copernicus_
variables, covariate_columns, product_url, forecast_variables,
climate_indices, and grid_resolution. They now have a "Looking things
up" section rather than a weakened test.
Paths prefer the source tree over the installed copy, so a local run
checks what is about to be committed. R CMD check has no source tree and
falls through to the installed files, which came from the tarball being
checked and are current too - which is why these run rather than skip
under check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Everything this package returns comes from someone else's data, and the obligation to cite travels with the data rather than with the package. The README had citations for the two indices published with papers and nothing for the Copernicus products, ETOPO, or the software - so someone publishing from a run had no list to work from. A References section at the bottom now covers all of it, grouped so only what a run actually used needs citing. Every DOI came from an authoritative source rather than from memory: the Copernicus ones from `copernicusmarine describe`, the package ones from citation(), and ETOPO from NCEI. All twelve were then checked to resolve. Two came back wrong. The Copernicus Marine Toolbox DOI I first wrote was a 404 - recalled, not looked up. There is no DOI for the client, so the section now points at its documentation and says to cite the products. More seriously, the AMOC reference in the catalog cited 10.5285/223b34a3-..., which also 404s. BODC mints a DOI per RAPID release and retires the old one; that was the 2004-2020 version. Now 10.5285/48d0bf43-... for v2024.1a, with the full author list, verified to resolve. A test pins it, since a dead DOI looks like a citation and sends the reader nowhere - worse than having none. Also adopts Copernicus's own required citation form, which asks for an access date, and adds a test that every data source has a DOI or a named provider in the reference list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two things, both about documentation staying true rather than the code being correct.
The test suite was not running under check
tests/testthat.Rhadtest_check("datamatch")commented out. It was commented on 23 July in a commit titled "update readme" — almost certainly a debugging leftover never restored.Since then
R CMD checkhas reportedchecking tests ... OKwhile running nothing, and CI has done the same. Every green CI run today included no tests.The suite has been passing throughout — my reports of that came from
testthat::test_local(), which does run it — but nothing was enforcing it. A regression would have reached master unnoticed.Uncommented. 759 tests now run under check, 0 failures, 0 skips.
Tests that the documentation keeps up
Guarding the failure mode that keeps recurring here: code lands, the documentation describing it does not, and nothing notices. Seven exported functions were once implemented, tested, and absent from the README entirely.
DESCRIPTIONdoes not still describematchData()as species-specific, and does mention what the package grewNEWS.mdhas a version heading R can parse — what stops theNo news entries foundNOTE coming backThe first one failed on its first run and found six:
copernicus_variables,covariate_columns,product_url,forecast_variables,climate_indices,grid_resolution. They now have a Looking things up section in the README, rather than a weakened test.On paths
They prefer the source tree over the installed copy, so a local run checks what is about to be committed.
R CMD checkhas no source tree and falls through to the installed files — which came from the tarball being checked, so they are current too. That is why these run rather than skip under check.An earlier version used relative paths only and errored under check when
read.dcf()gotNAbefore the skip guard. Caught by enablingtest_check()in the same change.Verification
R CMD check— Status: OK, with the suite actually executing. Installed into both R 4.6.1 and 4.3.2.🤖 Generated with Claude Code