Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package Testing and Vignette Building Within Temporary Directory #108

Merged
merged 36 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
3860f13
changes within core package
bburns632 Jun 13, 2018
78a467a
more specific paths within package
bburns632 Jul 11, 2018
a87c23b
test updates
bburns632 Jul 11, 2018
01569f3
revert .GetLibPaths
bburns632 Jul 27, 2018
432b6ca
start of withr::with_libpaths approach
bburns632 Jul 27, 2018
58a5854
working version
bburns632 Jul 29, 2018
65e7433
revert again GetLibPaths
bburns632 Jul 29, 2018
7141667
vignette edits
bburns632 Jul 30, 2018
fbbadaf
revised vignette approach
bburns632 Jul 31, 2018
60b536c
added test function
bburns632 Jul 31, 2018
ddae8f8
remove vignettes_source folder
bburns632 Jul 31, 2018
12b6dc6
testthat.R
bburns632 Aug 1, 2018
0f4e8c0
libpaths changes
bburns632 Aug 1, 2018
17beeab
revised handling of temp libpaths
bburns632 Aug 2, 2018
4cfcb8b
still fails R CMD CHECK
bburns632 Aug 6, 2018
40b3473
working on R CMD CHECK and devtools::check
bburns632 Aug 16, 2018
5e7c3b0
cleaned comments and docs
bburns632 Aug 17, 2018
6453424
wrap paths in testthat.R script
bburns632 Aug 20, 2018
8228f3b
again
bburns632 Aug 20, 2018
b40cef0
something else for paths
bburns632 Aug 20, 2018
b65adfc
maybe another path for travis
bburns632 Aug 20, 2018
04a611c
remove source statements in test
bburns632 Aug 21, 2018
d2dbe55
fixed empty test
jameslamb Sep 15, 2018
3d55c34
still trying to get past travis
jameslamb Sep 15, 2018
67bdfc0
still trying to figure out R CMD CHECK stuff
jameslamb Sep 16, 2018
caf2dcd
changed local install strategy
jameslamb Nov 3, 2018
313bbd0
omg this might work
jameslamb Nov 3, 2018
6bde9f8
fixed some misc R CMD CHECK stuff with NAMESPACE
jameslamb Nov 3, 2018
2df7b25
this works but has hard-coded stuff
jameslamb Nov 4, 2018
f5896aa
updated gitignore
jameslamb Nov 4, 2018
c067181
ok now it might actually be working
jameslamb Nov 4, 2018
bc05824
Fixed path resolution for non-CHECK testing
jameslamb Nov 4, 2018
54406f7
cleaned up a bit and added notes on how to run tests
jameslamb Nov 4, 2018
4063569
add vignettes_source back
jameslamb Nov 5, 2018
1eb876e
removed overly-specific directory assumptions in test.sh
jameslamb Nov 6, 2018
5255c05
uncommented browseURL code
jameslamb Nov 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,14 @@

# Misc files
.Rhistory
tests/testthat/Rplots.pdf
**/.DS_Store

# data files
**/*.rds
**/*.rda
**/*.csv

# build artifacts
**/*.tar.gz
pkgnet.Rcheck/*
10 changes: 10 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The goal of this guide is to help you contribute to `pkgnet` as quickly and as e
1. [Creating an Issue](#issues)
2. [Submitting a Pull Request](#prs)
3. [Code Style](#style)
4. [Running Tests Locally](#testing)

## Creating an Issue <a name="issues"></a>

Expand Down Expand Up @@ -235,3 +236,12 @@ RandomNumberPlotter <- R6::R6Class(

All comments should be above code, not beside it.

## Running Tests Locally <a name="testing"></a>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


We use Travis CI to automatically run unit tests and a serious of other automated checks on every PR commit and merge to `master`. Every `pkgnet` release also goes through a battery of automated tests run on CRAN before becoming officially available through CRAN.

However, these options can lengthen your testing cycle and make the process of contributing tedious. If you wish to run tests locally on whatever machine you are developing `pkgnet` code on, run the following from the repo root:

```{bash}
./test.sh
```
3 changes: 2 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ Imports:
visNetwork
Suggests:
devtools,
testthat
testthat,
withr
License: BSD_3_clause + file LICENSE
URL: https://github.com/UptakeOpenSource/pkgnet
BugReports: https://github.com/UptakeOpenSource/pkgnet/issues
Expand Down
3 changes: 3 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ export(DependencyReporter)
export(FunctionReporter)
export(SummaryReporter)
import(data.table)
importFrom(DT,datatable)
importFrom(DT,formatRound)
importFrom(R6,R6Class)
importFrom(assertthat,assert_that)
importFrom(assertthat,is.readable)
importFrom(assertthat,is.string)
importFrom(assertthat,is.writeable)
importFrom(covr,package_coverage)
importFrom(data.table,as.data.table)
importFrom(data.table,copy)
importFrom(data.table,data.table)
Expand Down
11 changes: 6 additions & 5 deletions R/PackageDependencyReporter.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#' its other package dependencies, determining which package it relies on is most central,
#' allowing for a developer to determine how to vet its dependency tree
#' @importFrom data.table data.table setnames rbindlist
#' @importFrom DT datatable formatRound
#' @importFrom R6 R6Class
#' @importFrom utils installed.packages
#' @importFrom tools package_dependencies
Expand Down Expand Up @@ -44,11 +45,11 @@ DependencyReporter <- R6::R6Class(
},

get_summary_view = function(){

# Calculate network measures if not already done
# since we want the node measures in summary
invisible(self$network_measures)

# Create DT for display
tableObj <- DT::datatable(
data = self$nodes
Expand Down Expand Up @@ -107,7 +108,7 @@ DependencyReporter <- R6::R6Class(

# Consider only installed packages when building dependency network
if (private$installed){
db <- utils::installed.packages()
db <- utils::installed.packages(lib.loc = .libPaths())
if (!is.element(self$pkg_name, db[,1])) {
msg <- sprintf('%s is not an installed package. Consider setting installed to FALSE.', self$pkg_name)
log_fatal(msg)
Expand Down Expand Up @@ -162,10 +163,10 @@ DependencyReporter <- R6::R6Class(
}

dependencyList <- Filter(function(x){!is.null(x)}, dependencyList)

if (identical(names(dependencyList), self$pkg_name)){
msg <- paste0(
"Package '%s' does not have any dependencies in [%s]. If you think this is an error ",
"Package '%s' does not have any dependencies in [%s]. If you think this is an error ",
"consider adding more dependency types in your definition of DependencyReporter. ",
"For example: DependencyReporter$new(dep_types = c('Imports', 'Depends', 'Suggests'))"
)
Expand Down
18 changes: 13 additions & 5 deletions R/PackageFunctionReporter.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
#' }
#' }
#' }
#' @importFrom covr package_coverage
#' @importFrom data.table data.table melt as.data.table data.table setnames setcolorder
#' @importFrom DT datatable formatRound
#' @importFrom mvbutils foodweb
#' @importFrom R6 R6Class
#' @importFrom utils lsf.str
Expand All @@ -33,11 +35,11 @@ FunctionReporter <- R6::R6Class(

public = list(
get_summary_view = function(){

# Calculate network measures if not already done
# since we want the node measures in summary
invisible(self$network_measures)

# Create DT for display
tableObj <- DT::datatable(
data = self$nodes
Expand Down Expand Up @@ -151,8 +153,12 @@ FunctionReporter <- R6::R6Class(
if (is.null(self$pkg_name)) {
log_fatal('Must set_package() before extracting nodes.')
}
nodes <- data.table::data.table(
node = as.character(unlist(utils::lsf.str(asNamespace(self$pkg_name))))
nodes <- data.table::data.table(node = as.character(
unlist(
utils::lsf.str(pos = asNamespace(self$pkg_name)
)
jameslamb marked this conversation as resolved.
Show resolved Hide resolved
)
)
)
return(nodes)
},
Expand All @@ -164,7 +170,9 @@ FunctionReporter <- R6::R6Class(

log_info(sprintf('Loading %s...', self$pkg_name))
suppressPackageStartupMessages({
require(self$pkg_name, character.only = TRUE)
require(self$pkg_name
, lib.loc = .libPaths()[1]
, character.only = TRUE)
})
log_info(sprintf('Done loading %s', self$pkg_name))

Expand Down
9 changes: 5 additions & 4 deletions R/PackageSummaryReporter.R
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
#' @title Package Summary Reporter Class
#' @name SummaryReporter
#' @family PackageReporters
#' @description Defines a concrete implementation of \link{AbstractPackageReporter}
#' @description Defines a concrete implementation of \link{AbstractPackageReporter}
#' for a high level overview of a particular package. It will summarize
#' things like lines of code, whether it's on CRAN, etc.
#' @inheritSection AbstractPackageReporter Public Methods
#' @importFrom DT datatable
#' @importFrom R6 R6Class
#' @export
SummaryReporter <- R6::R6Class(
classname = "SummaryReporter",
inherit = AbstractPackageReporter,
public = list(
get_summary_view = function(){

# Read DESCRIPTION file into a table
desc <- utils::packageDescription(self$pkg_name)
descDT <- data.table::data.table(
Field = names(desc)
, Values = unlist(desc)
)

# Render DT table
tableObj <- DT::datatable(
data = descDT
Expand All @@ -33,7 +34,7 @@ SummaryReporter <- R6::R6Class(
return(tableObj)
}
),

active = list(
report_markdown_path = function(){
system.file(file.path("package_report", "package_summary_reporter.Rmd"), package = "pkgnet")
Expand Down
2 changes: 1 addition & 1 deletion R/pkgnet.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ globalVariables(c('.'
#' @title NULL Object For Common Documentation
#' @description This is a NULL object with documentation so that later functions can call
#' inheritParams
NULL
NULL
71 changes: 71 additions & 0 deletions R/testing_utils.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@

# [Title] Build A Library For Testing
# [DESC] Installs all packages necessary for testing into a another directory,
# preferably a temporary directory. This function will throw a fatal error
# if that installation fails.
# [param] targetLibPath (string) path to the location of the new directory
# [return] boolean TRUE
.BuildTestLib <- function(targetLibPath){

### find PKGNET source dir within devtools::test(), R CMD CHECK, and vignette building
# NOTE: this can be fragile. Uncomment the lines with "# [DEBUG]" and run test.sh
# from the repo root if something goes wrong

# [DEBUG] write("=========", file = "~/thing.txt", append = TRUE)
# [DEBUG] write(list.files(getwd(), recursive = TRUE), file = "~/thing.txt", append = TRUE)
# [DEBUG] write(paste0("working dir: ", getwd()), file = "~/thing.txt", append = TRUE)

pkgnetSourcePath <- gsub('/pkgnet.Rcheck/tests/testthat$', replacement = '/pkgnet.Rcheck/00_pkg_src/pkgnet', x = getwd())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should circle back later (post merge), and see if there's a better way to do this as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I agree.

One thing that I think would be useful is to figure out if we can move this whole file out of the package source code. maybe store it in inst/ and source it.

pkgnetSourcePath <- gsub('/pkgnet.Rcheck/tests$', replacement = '/pkgnet.Rcheck/00_pkg_src/pkgnet', x = pkgnetSourcePath)
pkgnetSourcePath <- gsub('/pkgnet.Rcheck/vign_test/pkgnet$', replacement = '/pkgnet.Rcheck/00_pkg_src/pkgnet', x = pkgnetSourcePath)
pkgnetSourcePath <- gsub('/pkgnet/vignettes$', replacement = '/pkgnet', x = pkgnetSourcePath)
pkgnetSourcePath <- gsub('pkgnet/tests/testthat', replacement = 'pkgnet', x = pkgnetSourcePath)

# [DEBUG] write(paste0("pkgnet path: ", pkgnetSourcePath), file = "~/thing.txt", append = TRUE)
# [DEBUG] write("=========", file = "~/thing.txt", append = TRUE)

### packages to be built
pkgList <- c(
baseballstats = file.path(pkgnetSourcePath, "inst", "baseballstats")
, sartre = file.path(pkgnetSourcePath, "inst", "sartre")
, pkgnet = pkgnetSourcePath
)

### Install

# Figure out where R is to avoid those weird warnings about
# 'R' should not be used without a path -- see par. 1.6 of the manual.
#
# NOTE: R CMD CHECK comes with its own bundled "R" binary which doesn't
# work the same way and causes that error. Just trust me on this.
#
# NOTE: "Sys.which()" would be the correct, portable way to do this but it
# doesn't support matching ALL matches, so for now we'll make it work
# on unix-alike operating systems and deal with Windows later
#
R_LOC <- system("which -a R", intern = TRUE)
R_LOC <- R_LOC[!grepl("R_check_bin", R_LOC)][1]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayqi @bburns632 this part was the main culprit. Some other R executable gets bundled with R CMD CHECK but doesn't totally work and was the source of that dreaded see par 1.6 warning.

This thing I did here totally works (for both devtools::test() case and R CMD CHECK --as-cran case) but is not portable to Windows. We can figure that out later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱


# force install of SOURCE (not binary) in temporary directory for tests
cmdstr <- sprintf(
fmt = '"%s" CMD INSTALL -l "%s" %s'
, R_LOC
, targetLibPath
, paste0(pkgList, collapse = " ")
)

exitCode <- system(command = cmdstr, intern = FALSE)

if (exitCode != 0){

# Get the actual error text
output <- system(command = cmdstr, intern = TRUE)
stop(sprintf(
"Installation of packages in .BuildTestLib failed! (exit code = %s)\n\n%s"
, exitCode
, paste0(output, collapse = " ... ")
))
}

return(invisible(TRUE))
}
2 changes: 1 addition & 1 deletion man/SummaryReporter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/bash

rm *.tar.gz
rm ~/thing.txt
R CMD BUILD .

# Work outside of the source directory to avoid false
# positives (i.e. test the tarball in isolation)
mkdir -p ~/pkgnet_test_dir
cp *.tar.gz ~/Desktop

pushd ~/pkgnet_test_dir
R CMD CHECK *.tar.gz --as-cran
cat ~/thing.txt
popd
71 changes: 63 additions & 8 deletions tests/testthat.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,69 @@
# See https://github.com/hadley/testthat/issues/144
Sys.setenv("R_TESTS" = "")

library(pkgnet)
# Check if setup and helper funs have been run.
# If in R CMD CHECK, they may not have been run yet.
Sys.setenv(PKGNET_REBUILD = identical(Sys.getenv('PKGNET_TEST_LIB'), ''))

# Install Fake Packages - For local testing if not already installed
devtools::install_local(system.file('baseballstats',package="pkgnet"),force=TRUE)
devtools::install_local(system.file('sartre',package="pkgnet"),force=TRUE)
# If not yet run, rebuild
if (Sys.getenv('PKGNET_REBUILD')){
library(pkgnet)
## ******************************************************************************************
## THIS IS THIS SAME CONTENT as setup_setTestEnv.R but neccessary to paste here due to
## travis checks.
jameslamb marked this conversation as resolved.
Show resolved Hide resolved
## ******************************************************************************************

testthat::test_check('pkgnet')
# record original libpaths in order to reset later.
# This should be unnecessary since tests are conducted within a seperate enviornment.
# It's done out of an abundance of caution.
origLibPaths <- .libPaths()

# Uninstall Fake Packages - For local testing
devtools::uninstall(system.file('baseballstats',package="pkgnet"))
devtools::uninstall(system.file('sartre',package="pkgnet"))
# Set the pkgnet library for testing to a temp directory
Sys.setenv(PKGNET_TEST_LIB = tempdir())

# Set the libpaths for testing.
# This has no effect to global libpaths since testing tests are conducted within a
# seperate enviornment.
.libPaths(
new = c(Sys.getenv('PKGNET_TEST_LIB'), origLibPaths)
)

# Install Fake Packages - For local testing if not already installed
pkgnet:::.BuildTestLib(
targetLibPath = Sys.getenv('PKGNET_TEST_LIB')
)

}

# This withr statement should be redundant.
# This is within a test environment in which .libpaths() has been altered to include PKGNET_TEST_LIB.
# Yet, it appears to be necessary.
withr::with_libpaths(
new = .libPaths()
, code = {testthat::test_check('pkgnet')}
)

# Tear down temporary test enviorment
if (Sys.getenv('PKGNET_REBUILD')){

## ******************************************************************************************
## THIS IS THIS SAME CONTENT as teardown_setTestEnv.R but neccessary to paste here due to
## travis checks.
## ******************************************************************************************

# Uninstall Fake Packages From Test Library if Not Already Uninstalled
try(
utils::remove.packages(
pkgs = c('baseballstats', 'sartre', 'pkgnet')
, lib = Sys.getenv('PKGNET_TEST_LIB')
)
)

# Reset libpaths.
# This should be unnecessary since tests are conducted within a seperate enviornment.
# It's done out of an abundance of caution.
.libPaths(origLibPaths)

# Remove test libary path eviornment variable.
Sys.unsetenv('PKGNET_TEST_LIB')
}
20 changes: 20 additions & 0 deletions tests/testthat/setup_setTestEnv.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@

# record original libpaths in order to reset later.
# This should be unnecessary since tests are conducted within a seperate enviornment.
# It's done out of an abundance of caution.
origLibPaths <- .libPaths()

# Set the pkgnet library for testing to a temp directory
Sys.setenv(PKGNET_TEST_LIB = tempdir())

# Set the libpaths for testing.
# This has no effect to global libpaths since testing tests are conducted within a seperate enviornment.
.libPaths(new = c(
Sys.getenv('PKGNET_TEST_LIB')
, origLibPaths
))

# Install Fake Packages - For local testing if not already installed
pkgnet:::.BuildTestLib(
targetLibPath = Sys.getenv('PKGNET_TEST_LIB')
)
Loading