Raise test coverage from 18% (Groups A-D)#86
Merged
Conversation
Covers biocDownloadStats.R, condaDownloadStats.R, getData.R, and cache.R with real network requests against bioconductor.org, anaconda-download-stats, and the GitHub API (gated on skip_if_bioc_offline()/skip_if_offline()), plus pure-logic unit tests for the local caching and data-wrangling helpers that need no network at all. Also fixes a latent bug in the internal .get_all_biocpkgs() helper: it passed the *function* BiocManager::version (imported via NAMESPACE, shadowing base::version) as the `version` argument to BiocManager:::.repositories_bioc() instead of calling it, which made the helper error unconditionally. Changed to BiocManager::version(). The helper is currently unused elsewhere in the package but needed a working implementation to be testable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
) Covers biocExplore, biocPkgRanges, biocRevDepEmail (+ link-building helpers), biocBuildEmail (+ mail-log/user-info/template helpers), dataciteXMLGenerate, generateBiocPkgDOI, use_orcid.R, repositoryStats, CRANstatus, and getYearsInBioc.R. Per the tracking issue's testing philosophy, these prefer real requests against live Bioconductor/CRAN/ORCID resources (gated on skip_if_bioc_offline()/skip_if_offline()) over mocking. Functions with unsafe side effects (sending real email, minting real DOIs) are only exercised via their dry-run/text-only/sandbox-safe paths, or skipped with a clear explanation when credentials are unavailable (DATACITE_USERNAME/DATACITE_PASSWORD). getYearsInBioc.R tests use fabricated tiny manifests and single-version slices to avoid scanning the full multi-decade Bioconductor release history. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers R/biocPkgList.R CRAN-comparison helpers (CRAN_pkg_rds_url, get_CRAN_pkg_rds, read_CRAN_url, stripVersionString), R/buildPkgDependencyGraph.R, R/pkgDependencyMetrics.R, R/pkgBiocDeps.R, and R/classDependencies.R per the checklist in issue #85. Fixes the pkgBiocRevDeps() zero-coverage mystery: skip_if_offline() calls skip_on_cran() internally, which skips whenever NOT_CRAN isn't "true" -- true under devtools::test() but not under the standard tests/testthat.R runner used by R CMD check / covr, so the test was always skipped under coverage runs regardless of connectivity. Switched to skip_if_bioc_offline() (bioconductor.org reachability only, no skip_on_cran side effect). Applied the same reasoning to new CRAN-network tests in test_biocPkgList.R, using a direct curl::has_internet() check instead of skip_if_offline(). All dependency-graph/metrics tests use small, explicit package sets (e.g. GEOquery, Rarr, BiocPkgTools itself) rather than full-repository scans to keep runtime low; classDependencies.R tests need no network, exercising installed S4 classes (SummarizedExperiment, Biobase, S4Vectors) instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…githubUtils, utilities, biocBuildStatusDB Adds real-Bioconductor-data tests for problemPage()/checkMe()/checkDeps()/ chkURL() (issue #21 highest priority: checkMe() output is confirmed to include a real non-software (data-experiment) package with a live build problem, and that same real result is reused to validate problemPage()'s link-building/DT output, locking in the biocBuildReport() inner-join fix at this layer), biocMaintained()/hasBiocMaint(), the .read_VIEWS_url() error branch, .gh_pkg_info()/githubDetails() (gated on a working GitHub token via gh::gh_whoami(), skipping gracefully otherwise), remaining utilities.R gaps (.import_dcf_stage_node, .read_summary_dcfs, get_deprecated_status_df, .cache_url_file, .needs_update), and biocBuildStatusDB(). Also fixes a real bug in checkDeps() (R/problemPage.R) found while writing its test: `all_pkg_deps[cond, 1]` drops to an atomic vector, so the following `pkg_deps$Package` always threw "$ operator is invalid for atomic vectors" -- checkDeps()/problemPage(dependsOn = ...) has never actually worked. Fixed with `drop = FALSE`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Closes #85 (Group A-D test coverage push). generateBiocPkgDOI is left untested: the documented apitest/apitest sandbox login does not have write access to Bioconductor's reserved DataCite test prefix, so there is no credential-free way to exercise the live request path.
…y cache test R CMD check flagged undeclared 'tests' dependencies on withr (used via withr::) and Biobase (used via library()) on all platforms. Separately, write_to_cache()'s "errors when cache location unavailable" test assumes writing to filesystem root is denied, which doesn't hold on Windows CI runners that run elevated.
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.
Summary
Coordinated push to raise unit test coverage per the per-function checklist in #85, starting from 18.0% overall (
covr::package_coverage()). Adds/extends 28 test files (~2,344 lines) across four work groups:biocDownloadStats.R,condaDownloadStats.R,getData.R,cache.R)biocPkgList.R,buildPkgDependencyGraph.R,pkgDependencyMetrics.R,pkgBiocDeps.R,classDependencies.R)problemPage.R,biocMaintained.R,biocVIEWSdb.R,utilities.R,biocBuildStatusDB.R,githubUtils.R)biocExplore.R,biocPkgRanges.R,biocRevDepEmail.R,biocBuildEmail.R,dataciteXMLGenerate.R,use_orcid.R,repositoryStats.R,CRANstatus.R,getYearsInBioc.R)Testing philosophy: real requests against live Bioconductor/network resources where practical, gated on actual reachability (new
skip_if_bioc_offline()helper), rather than mocking.Two real production bugs were found and fixed while writing tests:
.get_all_biocpkgs()calledBiocManager:::.repositories_bioc(version = version)whereversionresolved to the importedBiocManager::versionfunction (not base R'sversionobject) due to NAMESPACE shadowing — this made the helper error unconditionally.checkDeps()subsetall_pkg_deps[cond, 1]dropped to an atomic vector, sopkg_deps$Packagealways threw —problemPage(dependsOn = ...)never actually worked. Fixed withdrop = FALSE.One test-infra bug was also fixed:
testthat::skip_if_offline()callsskip_on_cran()internally, so under the plaintestthat::test_check()runner used byR CMD check/covr (as opposed todevtools::test()), network tests were silently skipped regardless of real connectivity. Replaced with askip_if_bioc_offline()helper for bioconductor.org-specific tests.generateBiocPkgDOI(R/newBiocPkgDOI.R) remains untested — the one unchecked item on the #85 checklist. I evaluated whether this was fixable: the function's docs claimtesting = TRUEuses the sandboxapitest/apitestlogin, but the code always readsDATACITE_USERNAME/DATACITE_PASSWORDfrom the environment regardless of thetestingflag. I confirmed live thatapitest/apitestauthenticates againstapi.test.datacite.orgbut returns 404 when minting a DOI under Bioconductor's reserved test prefix (10.82962) — that prefix isn't associated with the generic sandbox account, only with real Bioconductor-issued credentials. So there's no credential-free way to exercise the live request-construction path, and no separable pure-logic piece to unit test without mocking. The existing test skips cleanly and runs for real whenever a maintainer/CI supplies the real credentials.Also bumps
Versionto 1.31.14 and adds a NEWS entry.Closes #85
Test plan
testthatsuite passes locally withNOT_CRAN=trueand live network reachablecovr::file_coverage()spot checks:pkgBiocDeps.R98.6%,buildPkgDependencyGraph.R97.4%,pkgDependencyMetrics.R90.0%,classDependencies.R94.2%R-CMD-check.yaml) green on this PR