diff --git a/DESCRIPTION b/DESCRIPTION index 7a263d0..9c6a898 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -31,6 +31,7 @@ Imports: Suggests: ggplot2, pkgdown, + pkgload, testthat, webshot, withr diff --git a/NEWS.md b/NEWS.md index 569985c..e4443e0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ ## CHANGES ## BUGFIXES +* `FunctionReporter` now determines exported functions from the package namespace's export metadata via `getNamespaceExports()` instead of listing the attached `package:` environment. Previously, running pkgnet after `devtools::load_all()` marked every function as exported. (#347 Thanks @hughjonesd!) # pkgnet 0.6.1 ## NEW FEATURES diff --git a/R/FunctionReporter.R b/R/FunctionReporter.R index 3a8fc96..9ea6e10 100644 --- a/R/FunctionReporter.R +++ b/R/FunctionReporter.R @@ -147,8 +147,10 @@ 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) + pkg_search_name <- paste0('package:', self$pkg_name) + reattach_pkg <- .Platform$OS.type == "windows" && pkg_search_name %in% search() + if (reattach_pkg) { + detach(pkg_search_name, unload = TRUE, character.only = TRUE) } pkgCovDT <- data.table::as.data.table(covr::package_coverage( @@ -158,7 +160,7 @@ FunctionReporter <- R6::R6Class( )) # workaround for covr conflict with loaded packages on windows - if(.Platform$OS.type == "windows") { + if (reattach_pkg) { attachNamespace(self$pkg_name) } @@ -236,13 +238,10 @@ FunctionReporter <- R6::R6Class( ) # Figure out which functions are exported - # We need the package to be loaded first - suppressPackageStartupMessages({ - require(self$pkg_name - , lib.loc = .libPaths()[1] - , character.only = TRUE) - }) - exported_obj_names <- ls(sprintf("package:%s", self$pkg_name)) + # Use the namespace's export metadata rather than the contents of the + # attached package environment. devtools::load_all() can attach + # unexported objects, which would otherwise look exported. + exported_obj_names <- getNamespaceExports(pkg_env) nodes[, isExported := node %in% exported_obj_names] # Check if we have R6 functions diff --git a/tests/testthat/test-FunctionReporter-class.R b/tests/testthat/test-FunctionReporter-class.R index d5d40d2..007eaca 100644 --- a/tests/testthat/test-FunctionReporter-class.R +++ b/tests/testthat/test-FunctionReporter-class.R @@ -390,3 +390,32 @@ test_that("FunctionReporter R6 edge extraction handles case where all methods ha TARGET = c("couplet_2", "couplet_1"),key = c("SOURCE","TARGET"))) }) + + +test_that("FunctionReporter determines exports from the namespace, not the attached environment", { + + testthat::skip_if_not_installed("pkgload") + + # Start from a clean slate so load_all() does not have to patch an + # already-loaded namespace + if (isNamespaceLoaded("silverstein")) { + unloadNamespace("silverstein") + } + + # devtools::load_all() copies the whole namespace into package:, so + # listing that environment makes every function look exported (#347) + pkgload::load_all( + path = system.file("silverstein", package = "pkgnet") + , quiet = TRUE + ) + on.exit(pkgload::unload("silverstein"), add = TRUE) + + # couplet_1 is not in silverstein's NAMESPACE, but load_all() attaches it + expect_true("couplet_1" %in% ls("package:silverstein")) + + testObj <- FunctionReporter$new()$set_package('silverstein') + + expect_false(testObj$nodes[node == "couplet_1", isExported]) + expect_false(testObj$nodes[node == "couplet_2", isExported]) + expect_true(testObj$nodes[node == "Carrots$public_methods$initialize", isExported]) +})