From d22fe77447522cc4f24c4d3da947bb601ddc53ca Mon Sep 17 00:00:00 2001
From: Mick Jordan <mick.jordan@oracle.com>
Date: Thu, 16 Mar 2017 14:42:15 -0700
Subject: [PATCH] pkgtest: fix bug when tested packages is already installed

---
 .../r/install.packages.R                      | 66 ++++++++++++-------
 documentation/dev/testing.md                  |  4 +-
 2 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/com.oracle.truffle.r.test.packages/r/install.packages.R b/com.oracle.truffle.r.test.packages/r/install.packages.R
index 2695cf973f..798ee89b0e 100644
--- a/com.oracle.truffle.r.test.packages/r/install.packages.R
+++ b/com.oracle.truffle.r.test.packages/r/install.packages.R
@@ -414,28 +414,34 @@ get.pkgs <- function() {
 
 	in.installed <- function(x) x["Package"] %in% installed.pkgs
 
-	basic.exclude <- function(x) {
-		in.installed(x) || in.blacklist(x)
+	basic.exclude <- function(x, exclude.installed = T) {
+		in.blacklist(x) || ifelse(exclude.installed, in.installed(x), F)
 	}
 
-	# either pkg.pattern is set or pkg.filelist but not both (checked earlier)
-	# if inverting, alter sense of the basic match but still exclude blacklist/installed
-	if (!is.na(pkg.filelistfile)) {
-		if (invert.pkgset) {
-			match.fun <- function(x) !in.filelist(x) && !basic.exclude(x)
-		} else {
-		    match.fun <- function(x) in.filelist(x) && !basic.exclude(x)
-	    }
-	} else {
-		if (invert.pkgset) {
-			match.fun <- function(x) !in.pattern(x) && !basic.exclude(x)
+	set.match.fun <- function(exclude.installed = T) {
+		# either pkg.pattern is set or pkg.filelist but not both (checked earlier)
+		# if inverting, alter sense of the basic match but still exclude blacklist/installed
+		if (!is.na(pkg.filelistfile)) {
+			if (invert.pkgset) {
+				match.fun <- function(x) !in.filelist(x) && !basic.exclude(x, exclude.installed)
+			} else {
+				match.fun <- function(x) in.filelist(x) && !basic.exclude(x, exclude.installed)
+			}
 		} else {
-		    match.fun <- function(x) in.pattern(x) && !basic.exclude(x)
+			if (invert.pkgset) {
+				match.fun <- function(x) !in.pattern(x) && !basic.exclude(x, exclude.installed)
+			} else {
+				match.fun <- function(x) in.pattern(x) && !basic.exclude(x, exclude.installed)
+			}
 		}
 	}
+
+	match.fun <- set.match.fun()
+
 	matched.avail.pkgs <- apply(avail.pkgs, 1, match.fun)
-	toinstall.pkgs <<-avail.pkgs[matched.avail.pkgs, , drop=F]
-	if (length(toinstall.pkgs) == 0) {
+	toinstall.pkgs <<- avail.pkgs[matched.avail.pkgs, , drop=F]
+
+	if (length(toinstall.pkgs) == 0 && !use.installed.pkgs) {
 		print("Fatal error: requested package(s) found in repo(s)")
 		quit(save="no", status=100)
 
@@ -450,15 +456,22 @@ get.pkgs <- function() {
 			test.pkgnames[[i]] <- test.avail.pkgnames[[rands[[i]]]]
 		}
 	} else {
-		test.pkgnames <- rownames(toinstall.pkgs)
-		if (!is.na(count.daily)) {
-			# extract count from index given by yday
-			npkgs <- length(test.pkgnames)
-			yday <- as.POSIXlt(Sys.Date())$yday
-			chunk <- as.integer(npkgs / count.daily)
-			start <- (yday %% chunk) * count.daily
-			end <- ifelse(start + count.daily > npkgs, npkgs, start + count.daily - 1)
-			test.pkgnames <- test.pkgnames[start:end]
+		if (length(toinstall.pkgs) == 0) {
+			# use.installed.pkgs == TRUE (see above)
+			match.fun <- set.match.fun(F)
+			matched.avail.pkgs <- apply(avail.pkgs, 1, match.fun)
+			test.pkgnames <- rownames(avail.pkgs[matched.avail.pkgs, , drop=F])
+		} else {
+			test.pkgnames <- rownames(toinstall.pkgs)
+			if (!is.na(count.daily)) {
+				# extract count from index given by yday
+				npkgs <- length(test.pkgnames)
+				yday <- as.POSIXlt(Sys.Date())$yday
+				chunk <- as.integer(npkgs / count.daily)
+				start <- (yday %% chunk) * count.daily
+				end <- ifelse(start + count.daily > npkgs, npkgs, start + count.daily - 1)
+				test.pkgnames <- test.pkgnames[start:end]
+			}
 		}
 	}
 
@@ -880,6 +893,9 @@ parse.args <- function() {
 	if (is.na(pkg.pattern) && is.na(pkg.filelistfile)) {
 	    pkg.pattern <<- "^.*"
 	}
+	if (!install) {
+		use.installed.pkgs <<- T
+	}
 	# list.versions is just that
     if (list.versions) {
 		install <<- F
diff --git a/documentation/dev/testing.md b/documentation/dev/testing.md
index 074302028c..d01e8d5dcd 100644
--- a/documentation/dev/testing.md
+++ b/documentation/dev/testing.md
@@ -104,7 +104,7 @@ Packages are downloaded and installed from the repos given by the `repos` argume
 The directory in which to install the package can be specified either by setting the `R_LIBS_USER` environment variable or with the `--lib` command line argument. The former is recommended and indeed required for running tests after installation (the testing system does not honor the `--lib` argument).
 
 ##### Specifying packages to Install
-If the `--pkg-filelist` argument is provided then the associated file should contain a list of packages to install, one per line. Otherwise if a package pattern argument is given, then all packages matching the (R) regular expression are candidates for installation, otherwise all available packages are candidates, computed by invoking the `available.packages()` function. The candidate set can be adjusted with additional options.  The `--use-installed-pkgs` option will cause `install.packages` to analyze the package installation directory for existing successfully installed packages and remove those from the candidate set. Some convenience options implicitly set `--pkg-filelist`, namely:
+If the `--pkg-filelist` argument is provided then the associated file should contain a list of packages to install, one per line. Otherwise if a package pattern argument is given, then all packages matching the (R) regular expression are candidates for installation, otherwise all available packages are candidates, computed by invoking the `available.packages()` function. The candidate set can be adjusted with additional options.  The `--use-installed-pkgs` option will cause `install.packages` to analyze the package installation directory for existing successfully installed packages and remove those from the candidate set for installation. This option is implied by `--no-install`. Some convenience options implicitly set `--pkg-filelist`, namely:
 
     --ok-only: sets it to the file `com.oracle.truffle.r.test.packages.ok.packages`. This file is a list of packages that are known to install.
 
@@ -143,7 +143,7 @@ Testing packages requires that they are first installed, so all of the above is
     --verbose | -v: output tracing on basic steps
     -V: more verbose tracing
     --dry-run: output what would be installed but don't actually install
-    --no-install | -n: suppress installation phase (useful for --create blacklist and --use-installed-pkgs/--run-tests)
+    --no-install | -n: suppress installation phase (useful for --create blacklist and --run-tests)
     --random count: install count packages randomly chosen from the candidate set
     --testdir dir: store test output in dir (defaults to "test").
     --print-ok-installs: print the successfully installed packages
-- 
GitLab