Skip to content

[R-pkg-devel] R CMD Check: Tests running infinite

14 messages · Uwe Ligges, Henrik Bengtsson, Hadley Wickham +2 more

#
Check whether the parallel cluster is closed. Can it be that the cluster 
is still open and the check process waits for them to complete?

Best,
Uwe Ligges
On 31.01.2017 13:45, Patrick Schratz wrote:
3 days later
#
Dear Uwe,

thanks for the hint. My cluster is closed after the `foreach`call using
`stopCluster()`.

Before, I?ll do the following to init the cluster:

*cl <- makeCluster(par.args$par.units, outfile = out.progress)*
*registerDoParallel(cl, cores = par.args$par.units)*

*foreach()*

*stopCluster(cl)*

Do you know of any other package which is using foreach in combination with
tests and is hosted on Github? So I could compare settings.

Best, Patrick

2017-02-02 0:01 GMT+01:00 Uwe Ligges <ligges at statistik.tu-dortmund.de>:

  
  
#
Use

  registerDoParallel(cl)

The number of parallel workers is already contained in the 'cl'
object, so don't
specify 'cores'!  (If you do that, I suspect you create yet another cluster
(a multicore one) which is used but never closed)

registerDoParallel() should ideally give an error in your case. Author
BCC:ed.

Henrik
On Feb 5, 2017 03:56, "Patrick Schratz" <patrick.schratz at gmail.com> wrote:

            

  
  
#
Thanks for the hint, Hendrik!
However, this change did not make a difference :/

I tried to use all cluster closing functions I came across but tests are
still running infinite..

*cl <- makeCluster(par.args$par.units, outfile = out.progress)*

*registerDoParallel(cl)*

*foreach()*

*parallel::stopCluster(cl)*
*doParallel::registerDoSEQ()*
*doParallel::stopImplicitCluster()*

2017-02-05 15:04 GMT+01:00 Henrik Bengtsson <henrik.bengtsson at gmail.com>:

  
  
#
@gaborcsardi solved it :) See here:
https://github.com/hadley/testthat/issues/567#issuecomment-277536577


2017-02-05 16:07 GMT+01:00 Patrick Schratz <patrick.schratz at gmail.com>:

  
  
#
In case someone else bumps into this later and finds this thread; can
you confirm that this was a problem specific to using the testthat
package for running the tests?

/Henrik

On Sun, Feb 5, 2017 at 11:28 AM, Patrick Schratz
<patrick.schratz at gmail.com> wrote:
#
I don't think it is specific for testthat. R CMD check sets R_TESTS when it
runs the tests, so the separate R process it starts can have some special
startup options.

The problem happens if you start another R process from your R test, and
then R_TESTS confuses this process. (I am not sure how exactly.)

A lot of packages have to work around this:
https://github.com/search?q=user%3Acran+R_TESTS&type=Code

Most of these use testthat, but not all of them.

Gabor

On Mon, Feb 6, 2017 at 2:17 AM, Henrik Bengtsson <henrik.bengtsson at gmail.com

  
  
1 day later
#
On Mon, Feb 6, 2017 at 12:41 AM, G?bor Cs?rdi <csardi.gabor at gmail.com> wrote:
I wasn't aware of R_TESTS.  Looking at the R source code, if set, will
cause the base package to source that file _before_ and .Rprofile
scripts; it basically calls source(Sys.getenv("R_TESTS")).

Next, R CMD check runs each of the test script using
R_TESTS=startup.Rs, where startup.Rs:

    file.copy(file.path(R.home("share"), "R", "tests-startup.R"), "startup.Rs")
    if (use_gct) cat("gctorture(TRUE)" , file = "startup.Rs", append = TRUE)

Looking at the tests-startup.R file that comes with my R 3.3.2 on
Ubuntu 16.04, it seems pretty harmless:

## A custom startup file for tests
## Run as if a system Rprofile, so no packages, no assignments
options(useFancyQuotes = FALSE)

So, to me it's not clear how this could make a difference in Patrick
case.   By disabling this, i.e. Sys.setenv(R_TESTS=""), I don't see
how it affects running parallel+foreach+doParallel.  Maybe because one
of those packages are relying on fancy quotes in some protocol,
passing command-line arguments or ... something.  I would be curious
to see what file.path(R.home("share"), "R", "tests-startup.R")
contains on the system where the problem occurs.
I wonder if those are mostly there because of cut'n'paste behavior.
My interest in this issue is because I haven't yet experienced this
myself and I run lots and lots of package tests in future that
utilizes the parallel package.  In doFuture I do similar tests, which
is on top of the foreach package.   I don't use testthat and I also
don't use doParallel in my testing.

/Henrik
#
It's not something I've ever advocated; I didn't realise there were so
many people unsetting it. (devtools does it for you)
The motivation for unsetting in in devtools is to avoid problems when
you run R CMD check from inside another R CMD check. I think it
affects some behaviour deep inside R, not just fancy quoting.

Hadley
#
On Tue, Feb 7, 2017 at 8:16 PM, Henrik Bengtsson <henrik.bengtsson at gmail.com
[...]
I don't know. What I know is that I have seen it in about 5 of my packages,
and I have also helped about 5 people that were stumbled upon it with their
own packages.

So it is definitely real. I also think that the workaround is fine. (Please
FIXME if not.) When Sys.unsetenv() is evaluated, it has already had its
effect.

G.

[...]
#
On Tue, Feb 7, 2017 at 8:16 PM, Henrik Bengtsson <henrik.bengtsson at gmail.com
[...]
I investigated a bit, and the problem (or at least one problem) is that the
R subprocess will try
to source() startup.Rs from the working directory, and that file might not
be there.
For testthat, the tests run in tests/testthat, and it is typically not
there.

For other regular tests, RUnit, etc. AFAIK the tests run in tests/ and
startup.Rs is
there. Unless the tests change the working directory, in which case it is
not.
So another workaround is to put an empty startup.Rs file there.

Maybe there are other problems as well, but just putting an empty
startup.Rs file at the appropriate
location fixed all cases I tried.

The parallel case above in this thread is a bit unusual I think, because
usually there
is no freeze, the R subprocess just aborts, and this typically causes a
testthat (or other) test
failure.

Nevertheless, putting an empty startup.Rs file in tests/testthat fixes the
testParallel
package as well.

G

  
  
#
Contents of mine on macOS are the same as on your Ubuntu:

## A custom startup file for tests
## Run as if a system Rprofile, so no packages, no assignments
options(useFancyQuotes = FALSE)

By disabling this, i.e. Sys.setenv(R_TESTS="")


Note that the fix from Gabor is Sys.unsetenv("R_TESTS"). What?s the
difference between both?

My interest in this issue is because I haven't yet experienced this
So if you use `foreach` and `parallel`, the major difference points to
`doParallel`. If I get my tests running by avoiding `doParallel`, we would
have some kind of evidence.
Any source on how to quickly substitute my `doParallel` code using
`parallel`? I know its just a few lines, but thats how it always starts..;)

2017-02-07 21:16 GMT+01:00 Henrik Bengtsson <henrik.bengtsson at gmail.com>:

  
  
#
Works for me as well!

2017-02-07 23:10 GMT+01:00 G?bor Cs?rdi <csardi.gabor at gmail.com>:

  
  
#
On Tue, Feb 7, 2017 at 2:10 PM, G?bor Cs?rdi <csardi.gabor at gmail.com> wrote:
Ah... that makes sence, because R CMD check passes/sets
R_TESTS=startup.Rs specifying only a relative pathname.  From runone()
part of src/library/tools/R/testing.R:

        if (WINDOWS) {
            Sys.setenv(LANGUAGE="C")
            Sys.setenv(R_TESTS="startup.Rs")
        } else
            cmd <- paste("LANGUAGE=C", "R_TESTS=startup.Rs", cmd)

Maybe a more robust version would be to use something like:

        if (WINDOWS) {
            Sys.setenv(LANGUAGE="C")
            Sys.setenv(R_TESTS=normalizePath("startup.Rs"))
        } else
            cmd <- paste("LANGUAGE=C", paste0("R_TESTS=",
normalizePath("startup.Rs")), cmd)

On the other hand, if it should be assumed that the test is run in the
same directory as startup.Rs, then testthat, and other test frameworks
that don't run under tests/, could copy startup.Rs as:

   local({
      fn <- Sys.getenv("R_TESTS")
      if (nzchar(fn) && file_test("-f", pn <- file.path("..", fn)))
file.copy(pn, fn)
   })


@Patrick,
My bad; Sys.unsetenv("R_TESTS") is the *correct* way to unset an env
variable.  My approach sets it to an empty string, which also happens
to work because the test to source the file or not is based on
nzchar(Sys.setenv("R_TESTS")) == TRUE.

/Henrik