Fix CI: commit test snapshots, drop the stats blanket import, roxygen2 8.1.0 - #36
Open
billdenney wants to merge 6 commits into
Open
billdenney wants to merge 6 commits into
billdenney wants to merge 6 commits into
Conversation
R-CMD-check and test-coverage failed on every platform for two independent reasons. Tests: `tests/testthat/_snaps` was listed in `.gitignore`, so none of the 23 snapshots were ever committed. testthat treats a missing snapshot as a warning locally but as a *failure* when `CI` is set, so all snapshot expectations in `test-calc.R`, `test-misc.R`, `test-nm.R` and `test-plot.R` failed on GitHub Actions, giving "checking tests ... ERROR" and the covr test failure. Un-ignore `_snaps` and commit the generated snapshots. vdiffr SVG snapshots record text metrics from the system "sans" font, which resolves to DejaVu Sans on Linux but Helvetica on macOS and Arial on Windows, so the committed figures can only be compared on Linux. Guard the two figure tests with `skip_on_os()`. Namespace: `@import stats` in `get_auc()` pulled in `stats::filter`, which `@importFrom dplyr filter` then replaced, emitting "replacing previous import" at load time. r-lib/actions/check-r-package uses `error-on: warning`, so that install warning alone failed the check. Replace the blanket import with `stats::` at the call sites. Also refresh the workflows with usethis (they were already current; only the R-CMD-check README badge was added) and regenerate the documentation with roxygen2 8.1.0, which replaces `RoxygenNote` with `Config/roxygen2/version`. R CMD check --as-cran now reports 0 errors and 0 warnings with `error_on = "warning"`, and covr completes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`.DS_Store` had no trailing newline, so appending the two testthat entries in the previous commit produced the pattern `.DS_Storetests/testthat/_problems` — which ignored neither `.DS_Store` nor the problems directory. Rewrite the file with a trailing newline so each pattern is on its own line. README carried a corrupted R-CMD-check badge whose image URL was repeated three times, and `usethis::use_github_action()` did not recognise it, so it appended a second one. Repair the original badge in place and drop the duplicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`@import utils` was the same defect as the `stats` one: a blanket import that can be silently overridden by a later `importFrom`. Replace it with `utils::` at the call sites (`count.fields`, `globalVariables`, `head`, `modifyList`, `read.table`) and drop the tag from `read_nmcov()`, which used no `utils` functions at all. `import(ggplot2)` is kept, as a blanket import is conventional for a plotting package. NEWS.md had no section for 1.6 even though the version was bumped in "Misc updates". Add one covering `cut_quantile()`, `datamap()`, the `dgr_table()` named-`fields` interface, the new `purrr` and `tidyr` dependencies, and the removal of the blanket imports. R CMD check --as-cran still reports 0 errors and 0 warnings with `error_on = "warning"`, with no new "no visible global function definition" notes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…able Two failures remained on PR CI after the snapshots were committed. read_nm() called `xml2::read_xml(fileName, ".xml", sep = "")`, a mangled `paste0()` refactor that passes ".xml" as libxml2's `encoding` argument. Linux and Windows ignore the bogus encoding silently, but macOS warns `Unsupported encoding: .xml [32]`, which was captured by `expect_snapshot()` and failed the read_nm, sample_omega and sample_sigma snapshots there. The same line also read the raw `fileName` rather than the `fileName_read` resolved by `check_file_exists()`, so `directory=` and extension-less names were ignored; `readLines()` above it had the same defect. sample_uncert() draws via `MASS::mvrnorm()`, which eigendecomposes the covariance matrix. THETA4 and OMEGA(2,1) are fixed in this run, so that matrix is singular and the eigenvectors spanning its null space are LAPACK-dependent — the draws for a fixed seed differ between Linux, macOS and Windows, so they cannot be snapshotted. Replace the snapshot with assertions on what is actually determinate: the shape and names, that the two fixed parameters are returned exactly, and that the column means and standard deviations match the estimates and the covariance diagonal. Ratios are compared so the tolerance applies per parameter instead of being swamped by V2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
macOS was the only platform left failing. THETA4 and OMEGA(2,1) have zero variance, and on Linux and Windows `MASS::mvrnorm()` returns them bit-identical, so `unique()` collapsed to a single value. macOS leaves rounding noise around 1e-10 from the eigendecomposition, so `unique()` returned 5000 distinct values just off the estimate. Compare the full column against the estimate with a tolerance instead, which still pins the values roughly four orders of magnitude tighter than the smallest parameter that does vary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The mangled `xml2::read_xml()` call meant `directory=` and extension-less file names were silently ignored, so this is a user-facing fix rather than a test-only one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
@kestrel99, Can you please take a quick look? While large, other than the |
This branch has not been deployed
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.
R-CMD-checkandtest-coveragehave failed on every platform since the newworkflows landed. There were two independent causes; neither was the workflow
configuration.
1. Test snapshots were never committed
.gitignorelistedtests/testthat/_snaps, so none of the 23 snapshots existedon CI. testthat treats a missing snapshot as a warning locally but as a
failure when the
CIenvironment variable is set — which is exactly whythis passed locally and failed on GitHub Actions.
Reproduced with:
CI=true NOT_CRAN=true Rscript -e 'testthat::test_local()'23 failures across
test-calc.R,test-misc.R,test-nm.Randtest-plot.R,matching both
checking tests ... ERRORin R-CMD-check andrunning the tests in 'testthat.R' failedin test-coverage.Fix: un-ignore
_snapsand commit the snapshots.The vdiffr SVG snapshots embed
textLengthvalues computed from the systemsansfont, which resolves to DejaVu Sans on Linux, Helvetica on macOS andArial on Windows — so committed baselines can only be compared on one platform.
The two figure tests are now guarded with
skip_on_os(c("mac", "windows")).2.
@import statscollided withdplyr::filterget_auc()carried a blanket@import stats.importFrom(dplyr, filter)thenreplaced
stats::filter, emittingreplacing previous import 'stats::filter' by 'dplyr::filter' when loading 'pmxTools'at load time.r-lib/actions/check-r-packageruns witherror-on: warning, so that installwarning alone failed the check even independently of the test failures.
Fix: drop the blanket import and use
stats::at the 19 call sites(
aggregate.data.frame,median,pchisq,pnorm,qnorm,quantile,reshape,sd,setNames).Also in this commit
usethis::use_github_action(). They werealready up to date with the r-lib/actions v2 examples — the only resulting
change was the R-CMD-check badge added to
README.md.RoxygenNote: 7.3.3withConfig/roxygen2/version: 8.1.0and rewrites\link{f}()cross-references as\link[=f]{f()}.Verification
Run locally under CI conditions (
CI=true,NOT_CRAN=true,_R_CHECK_CRAN_INCOMING_=false,_R_CHECK_FORCE_SUGGESTS_=false):rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran"), error_on = "warning")→ 0 errors, 0 warnings. The single remaining NOTE is
.gitbeing a filein the git worktree used for development; it does not occur in a normal
checkout.
covr::package_coverage()completes: 65.06%.Follow-ups not included here
import(utils)is the same class of blanket import as thestatsone fixedhere (6 functions across 5 files) and could be converted the same way.
import(ggplot2)is left as a conventional blanket