Core Function Changes to Accommodate Empty Lists in Recursion, Covr Behavior on Windows & Vignette Path Warnings

.github/workflows/release.yml

@@ -79,7 +79,7 @@ jobs:
         with:
           fetch-depth: 1
       - name: If local, apt update
-        if: ${ (env.ACT || false)}
+        if: ${{ (env.ACT || false)}}
         run: sudo apt update
       - name: Install Tidy Ubuntu
         run: sudo apt install -y tidy

NEWS.md

@@ -5,8 +5,11 @@
 
 ## CHANGES
 * Updated `pkgnet-intro` vignette to include information on the Class Inheritance Reporter and other minor edits. 
+* Recursive function network creation made tolerant to empty lists. (#322)
+* Excessive warnings removed for custom `vignette_path` param in `CreatePackageVignette()` (#322)
 
 ## BUGFIXES
+* `CreatePackageReporter()` failing on Windows to build package coverage when `report_path` specified. (#322) 
 
 # pkgnet 0.4.2
 ## NEW FEATURES

R/CreatePackageVignette.R

@@ -85,42 +85,6 @@ CreatePackageVignette <- function(pkg = "."
                           , dirname(vignette_path)))
     }
 
-    # Check if vignette_path matches the right package
-    # if the path is to a file in a directory named vignettes
-    vignetteDirAbsPath <- normalizePath(dirname(vignette_path))
-    # If path is a vignettes directory
-    if (grepl('/vignettes$', vignetteDirAbsPath)) {
-        # Get path for expected DESCRIPTION file for package
-        expectedDescriptionPath <- gsub(
-            pattern = "vignettes$"
-            , replacement = "DESCRIPTION"
-            , x = vignetteDirAbsPath
-            )
-
-        # If DESCRIPTION file exists check the name
-        if (file.exists(expectedDescriptionPath)) {
-            foundPkgName <- read.dcf(expectedDescriptionPath)[1,][["Package"]]
-
-            # If it doesn't match pkg_name, give warning
-            if (!identical(foundPkgName, pkg_name)) {
-                log_warn(glue::glue(
-                    "You are writing a report for {pkg_name} to the vignettes "
-                    , "directory for {foundPkgName}"
-                    , pkg_name = pkg_name
-                    , foundPkgName = foundPkgName))
-            }
-
-        # Otherwise, warn that we're writing to a vignettes folder inside
-        # a directory that is not a package root
-        } else {
-            log_warn(paste(
-                "You specified a path to a vignettes directory"
-                , vignetteDirAbsPath
-                , "that is not inside a package root directory."
-            ))
-        }
-    }
-
     log_info(sprintf(
         "Creating pkgnet package report as vignette for %s..."
         , pkg_name

R/FunctionReporter.R

@@ -151,12 +151,22 @@ FunctionReporter <- R6::R6Class(
 
             log_info(sprintf("Calculating test coverage for %s...", self$pkg_name))
 
+            # workaround for covr conflict with loaded packages on windows
+            if(.Platform$OS.type == "windows") {
+                detach(paste0('package:',self$pkg_name), unload = TRUE, character.only = TRUE)
+            }
+
             pkgCovDT <- data.table::as.data.table(covr::package_coverage(
                 path = private$pkg_path
                 , type = "tests"
                 , combine_types = FALSE
             ))
 
+            # workaround for covr conflict with loaded packages on windows
+            if(.Platform$OS.type == "windows") {
+                attachNamespace(self$pkg_name)
+            }
+
             pkgCovDT <- pkgCovDT[, .(coveredLines = sum(value > 0)
                                     , totalLines = .N
                                     , coverageRatio = sum(value > 0)/.N
@@ -395,12 +405,17 @@ FunctionReporter <- R6::R6Class(
     if (!is.list(x) && listable) {
         x <- as.list(x)
 
-        # Check for expression of the form foo$bar
-        # We still want to split it up because foo might be a function
-        # but we want to get rid of bar, because it's a symbol in foo's namespace
-        # and not a symbol that could be reliably matched to the package namespace
-        if (identical(x[[1]], quote(`$`))) {
-            x <- x[1:2]
+        if (length(x) > 0){
+            # Check for expression of the form foo$bar
+            # We still want to split it up because foo might be a function
+            # but we want to get rid of bar, because it's a symbol in foo's namespace
+            # and not a symbol that could be reliably matched to the package namespace
+            if (identical(x[[1]], quote(`$`))) {
+                x <- x[1:2]
+            }
+        } else {
+            # make empty lists "not listable" so recursion stops
+            listable <- FALSE 
         }
     }
 
@@ -643,9 +658,10 @@ FunctionReporter <- R6::R6Class(
     if (!is.list(x) && listable) {
         xList <- as.list(x)
 
+        if (length(xList) > 0){
+
         # Check if expression x is from _$_
         if (identical(xList[[1]], quote(`$`))) {
-
             # Check if expression x is of form self$foo, private$foo, or super$foo
             # We want to keep those together because they could refer to the class'
             # methods. So expression is not listable
@@ -654,6 +670,7 @@ FunctionReporter <- R6::R6Class(
                 || identical(xList[[2]], quote(super))) {
                 listable <- FALSE
 
+
             # If expression lefthand side is not keyword, we still want to split
             # it up because left might be a function
             # but we want to get rid of right, because it's a symbol in left's namespace
@@ -667,8 +684,15 @@ FunctionReporter <- R6::R6Class(
         } else {
             x <- xList
         }
+        } else {
+            # make empty list "non-listable" so recursion stops
+            listable <- FALSE
+        }   
+
     }
 
+
+
     if (listable){
         # Filter out atomic values because we don't care about them
         x <- Filter(f = Negate(is.atomic), x = x)

tests/testthat/test-CreatePackageVignette.R

@@ -174,51 +174,3 @@ test_that("Test that CreatePackageVignette errors for bad inputs", {
         , fixed = TRUE
     )
 })
-
-test_that("CreatePackageVignette warns if vignette_path seems wrong", {
-
-    pkgPath <- .CreateSourceCopy(sourcePath)
-    on.exit(expr = unlink(pkgPath, recursive = TRUE))
-
-    # In a vignettes directory that isn't in a package root
-    vignettesDir <- file.path(tempdir(), "vignettes")
-    dir.create(vignettesDir)
-    expect_warning(
-        CreatePackageVignette(pkg = pkgPath
-                              , vignette_path = file.path(vignettesDir
-                                                          , "pkgnet_report.Rmd")
-        )
-        , regexp = paste("not inside a package root directory")
-        , fixed = TRUE
-    )
-    # Clean up
-    unlink(file.path(tempdir(), "vignettes"), recursive = TRUE)
-
-    # If in root of a different package
-    suppressWarnings({
-        # creating a temporary environment to avoid the following error from package.skeleton()
-        # "... no R objects specified or available"
-        basketball_env <- new.env()
-        basketball_env[["a"]] <- function(){return(1)}
-        utils::package.skeleton(
-            name = "basketballstats"
-            , environment = basketball_env
-            , path = tempdir()
-        )
-    })
-    dir.create(file.path(tempdir(), "basketballstats", "vignettes"))
-    expect_warning(
-        CreatePackageVignette(pkg = pkgPath
-                              , vignette_path = file.path(tempdir()
-                                                          , "basketballstats"
-                                                          , "vignettes"
-                                                          , "pkgnet_report.Rmd")
-        )
-        , regexp = paste("You are writing a report for baseballstats to the"
-                         , "vignettes directory for basketballstats")
-        , fixed = TRUE
-    )
-    # Clean up
-    unlink(file.path(tempdir(), "basketballstats"), recursive = TRUE)
-
-})