Apply suggestions from code review

Co-authored-by: Chris Black <[email protected]>
PecanProject · Oct 9, 2024 · 8f820b0 · 8f820b0
1 parent f7f6926
commit 8f820b0
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 8 deletions.
diff --git a/base/db/R/check.missing.files.R b/base/db/R/check.missing.files.R
@@ -1,7 +1,6 @@
 #' Function to check if result has empty or missing files
 #'
 #' @param result A list of dataframes with file paths
-#' @param outname Name of the output file
 #' @param existing.input Existing input records
 #' @param existing.dbfile Existing dbfile records
 #' @return A list of dataframes with file paths, a list of strings with the output file name, a list of existing input records, and a list of existing dbfile records

diff --git a/base/db/R/convert_input.R b/base/db/R/convert_input.R
@@ -380,7 +380,7 @@ convert_input <-
     if (!is.null(ensemble) && ensemble) {
       return.all <-TRUE
 
-    } else{
+    } else {
       return.all <- FALSE
     }
       existing.dbfile <- dbfile.input.check(siteid = site.id,
@@ -518,6 +518,10 @@ convert_input <-
   # Get machine information
   machine.info <- get.machine.info(host, input.args = input.args, input.id = input.id)
 
+  if (any(sapply(machine.info, is.null))) {
+    PEcAn.logger::logger.error("failed lookup of inputs or dbfiles")
+    return(NULL)  
+  }
   machine <- machine.info$machine
   input <- machine.info$input
   dbfile <- machine.info$dbfile

diff --git a/base/db/R/get.machine.info.R b/base/db/R/get.machine.info.R
@@ -18,7 +18,7 @@ get_machine_info <- function(host, input.args, input.id = NULL, con = NULL) {
     return(NULL)
   }
 
-  if (missing(input.id) || is.na(input.id) || is.null(input.id)) {
+  if (is.na(input.id) || is.null(input.id)) {
     input <- dbfile <- NULL
   } else {
     input <- db.query(paste("SELECT * from inputs where id =", input.id), con)
@@ -71,7 +71,7 @@ get_machine_info <- function(host, input.args, input.id = NULL, con = NULL) {
 #' @param con database connection
 #' @return list of machine host and machine information
 #' @author Abhinav Pandey
-get.machine.host <- function(host, con = NULL) {
+get_machine_host <- function(host, con) {
   #Grab machine info of host machine
   machine.host <- ifelse(host$name == "localhost", PEcAn.remote::fqdn(), host$name)
   machine <- db.query(paste0(

diff --git a/base/db/tests/testthat/test.check.missing.files.R b/base/db/tests/testthat/test.check.missing.files.R
@@ -1,16 +1,14 @@
 test_that("`check_missing_files()` able to return correct missing files", {
     # Mock `purrr::map_dfr`
-    mocked_res <- mockery::mock(data.frame(file = c("A", "B"), file_size = c(100, 200), missing = c(FALSE, FALSE), empty = c(FALSE, FALSE)))
-    mockery::stub(check_missing_files, "purrr::map_dfr", mocked_res)
+    mocked_size <- mockery::mock(100,200)
+    mockery::stub(check_missing_files, "file.size", mocked_res)
 
     res <- check_missing_files(
         result = list(data.frame(file = c("A", "B"))),
         existing.input = data.frame(),
         existing.dbfile = data.frame()
     )
 
-    # Print the structure of `res` for debugging
-    str(res)
 
     expect_equal(length(res), 2)
     expect_true(is.list(res[[1]]))