Fix check.bety.version unknown-migration check

CHANGELOG.md

@@ -71,6 +71,7 @@ For more information about this file see also [Keep a Changelog](http://keepacha
 - Fixed `web/08-finished.php`: show database info instead of "Still running" when workflow folder doesn't exist locally (#3501).
 - `PEcAn.utils::transformstats()`: corrected the LSD-to-SE conversion. The previous implementation included an extra `sqrt(n)` factor, causing SE estimates derived from LSD to appear `sqrt(n)` times smaller than they should be, non-conservatively over-weighting those observations in meta-analysis. (#3998)
 - `segment_dataframe()` now returns an empty dataframe when date filtering removes all crop-cycle segments, instead of a single row with NA columns that caused downstream segment config errors (#4007).
+- `PEcAn.settings::check.bety.version()` now warns about database migrations unknown to this release; previously `any()` was applied to a character vector, which raised a coercion warning followed by a "missing value where TRUE/FALSE needed" error.
 
 ### Changed
 

base/settings/NEWS.md

@@ -10,6 +10,7 @@
 * Breaking: `loadPath.SitePFT(path)` now takes only one argument, removing the previously required but ignored `settings` (@Thomas-Sedhom, #3834).
 * `check.model.settings()` now skips lookup of the model binary path if no DB connection is provided (#3863).
 * When no DB connection is provided to `check.database.settings`, it now skips with an INFO instead of a WARN. Rationale: now that the database is optional, a settings with no DB block is a valid settings.
+* `check.bety.version()` now emits its intended warning when the database has migrations unknown to this release. Previously it called `any()` on a character vector, which produced a coercion warning and then an error.
 * Added regression tests confirming `check.ensemble.settings()` does not add an `ensemble` block to settings that lack one, including sensitivity-analysis-only runs (#3024).
 
 

base/settings/R/check.all.settings.R

@@ -252,7 +252,7 @@ check.bety.version <- function(dbcon) {
 
   # check if database is newer
   unknown_migrations <- setdiff(versions, .known_bety_migrations)
-  if (any(unknown_migrations)) {
+  if (length(unknown_migrations) > 0) {
     PEcAn.logger::logger.warn(
       "Found database migration(s) not known by this release of PEcAn.settings:",
       unknown_migrations,

base/settings/tests/testthat/test.check.bety.version.R

@@ -62,3 +62,31 @@ test_that("`check.bety.version`` gives errors for missing significant versions",
   expect_silent(check.bety.version(dbcon))
 })
 
+
+test_that("`check.bety.version` warns, without error, about migrations unknown to this release", {
+  known <- c("1", "20140617163304", "20140708232320", "20140729045640", "20151011190026")
+
+  warn_mock <- mockery::mock(NULL)
+  mockery::stub(
+    check.bety.version,
+    "PEcAn.DB::db.query",
+    list(version = c(known, "99999999999999"))
+  )
+  mockery::stub(check.bety.version, "PEcAn.logger::logger.warn", warn_mock)
+  expect_no_error(check.bety.version(1))
+  mockery::expect_called(warn_mock, 1)
+  expect_match(
+    paste(mockery::mock_args(warn_mock)[[1]], collapse = " "),
+    "99999999999999"
+  )
+})
+
+test_that("`check.bety.version` does not warn when all migrations are known", {
+  known <- c("1", "20140617163304", "20140708232320", "20140729045640", "20151011190026")
+
+  warn_mock <- mockery::mock(NULL)
+  mockery::stub(check.bety.version, "PEcAn.DB::db.query", list(version = known))
+  mockery::stub(check.bety.version, "PEcAn.logger::logger.warn", warn_mock)
+  check.bety.version(1)
+  mockery::expect_called(warn_mock, 0)
+})