Timeline for package checks in CI (+ list of current check errors)

[infotroph]

Describe the bug
Current Travis builds run unit tests (devtools::test) on each package in PEcAn, but do not run any package checks (R CMD check or near-equivalently devtools::check), which would run tests and check for many possible errors in package structure, code quality, and documentation completeness.

I propose to implement full package checks on a four-stage schedule:

• Immediately:
  • begin running checks on all packages every Travis build
  • Exempt those that cause the build to fail, using a temporary whitelist in .travis.yml. Continue running devtools::test on these.
• By September 1:
  • Correct all ERRORs in check output, removing packages from whitelist as we go
  • Once whitelist is empty, all packages will be checked every build and an ERROR from the check will break the build.
  • WARNINGs in check output will not yet break the build, but should have open issues to track them.
• Starting 2019:
  • Begin treating WARNINGs in check output as errors.
  • Packages that produce warnings are exempted using a temporary whitelist. Continue checking these and breaking the build on ERRORs.
• By PEcAn 2.0:
  • Travis treats WARNINGs in check output as build-breaking errors.

To Reproduce

Packages that currently produce at least one ERROR from package checks (As determined from this experimental Travis build):

• base/db: Fails with two check errors: One syntax error in examples, one test failure that doesn't occur in devtools::test() on this Travis build or at all (devtools::test, devtools::check, or R CMD build && R CMD check) on my machine. To correct, fix example syntax then look for differences in dependency calculation between $(make document) and $(make install).

• base/utils: Fails with "Packages suggested but not available: ‘PEcAn.data.atmosphere’ ‘PEcAn.data.land’ ‘PEcAn.emulator’ ‘PEcAn.priors’ ‘PEcAn.settings’ ‘PEcAn.DB’". To correct, need to finish fixing circular dependencies so these can be removed from Suggests in base/utils.

• models/ed: Fails with check error "Namespace dependency not required: ‘magrittr’". To correct, add magrittr to Imports.

• models/linkages: Fails with check error "Package suggested but not available: ‘linkages’". To correct: fix syntax in Remotes field

• modules/preles: Fails with check error "Package suggested but not available: ‘Rpreles’". To correct: add to Remotes field (Is https://github.com/MikkoPeltoniemi/Rpreles the correct remote address?)

• modules/benchmark: Fails with check error "Namespace dependency not required: ‘PEcAn.utils’". To correct: Remove outdated importFrom(PEcAn.utils, logger.info) from NAMESPACE

• modules/data.land: Fails with test error "Could not connect to database. RS-DBI driver: (could not connect bety@localhost:5432 on dbname "fia5data": FATAL: database "fia5data" does not exist ) postgresqlNewConnection(drv, ...) "

• modules/meta.analysis: Fails with test error "approx.posterior function returns expected dataframe given test dat\n no 'dimnames' attribute for array 1: approx.posterior(trait.mcmc, priors = prior.distns) at testthat/test.approx.posterior.R:28". Note that error is in tests, but only reported by check, not test. Don't know why.

Machine (please complete the following information):

• Server: Travis
• OS: Linux
• Browser(if applicable): NA
• Version: 1.5.3, branched from develop at 25bba2e

Additional context
Add any other context about the problem here.

[infotroph]

Update: travis now runs checks on all packages and breaks the build on any ERROR from the check process.

In the next phase, I will begin breaking the build for specific WARNINGs (e.g. die on "Undocumented arguments in documentation object" but continue succeeding on " Files in the 'vignettes' directory but no files in 'inst/doc'" for a while longer) until enough are fixed to ratchet up to breaking the build on any WARNING.

[mdietze]

Why do @infotroph PR make me feel like a 🐸 in a 🍵 🔥 ?

[infotroph]

@mdietze I aim more for 🚶 on a comfy, comfy Procrustean 🛏

[infotroph]

Closing because its original scope is complete: All packages are now checked at build time and report build failures when a change adds new problems. Cleaning up the pre-existing messages will remain ongoing for a while (see #2404 and the Rcheck_reference.log in each package's test/ directory for details) but should be tracked in separate issues.