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

TESTS: expect_true(inherits(plan(), 'sequential')) -> expect_true(identical(plan(), oplan)) #1

Closed
HenrikBengtsson opened this issue Sep 19, 2020 · 6 comments

Comments

@HenrikBengtsson
Copy link

Hi, I'm running reverse package dependency checks on future. I do this with the default settings as well as with env var R_FUTURE_PLAN=multisession, which will force the default future plan to be multisession rather than sequential. This will further validate that all packages work when running in parallel, e.g. all global variables are properly exported, etc.

This results in errors when checking your package because some of your tests assert at the end that sequential is still set. You can reproduce this yourself by running:

export R_FUTURE_PLAN=multisession
R CMD check codalm_0.1.0.tar.gz
...
* checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Complete output:
  > library(testthat)
  > library(codalm)
  > 
  > test_check("codalm")
  ── 1. Failure: bootstrap CI works with sequential evaluation (@test-codalm_ci.R#
  inherits(plan(), "sequential") isn't true.
  
  ── 2. Failure: bootstrap CI works with multisession evaluation (@test-codalm_ci.
  inherits(plan(), "sequential") isn't true.
  
  ── 3. Failure: independence test works with sequential evaluation (@test-indepen
  inherits(plan(), "sequential") isn't true.
  
  ── 4. Failure: independence test works with multisession evaluation (@test-indep
  inherits(plan(), "sequential") isn't true.
  
  ══ testthat results  ═══════════════════════════════════════════════════════════
  [ OK: 22 | SKIPPED: 0 | WARNINGS: 1 | FAILED: 4 ]
  1. Failure: bootstrap CI works with sequential evaluation (@test-codalm_ci.R#20) 
  2. Failure: bootstrap CI works with multisession evaluation (@test-codalm_ci.R#42) 
  3. Failure: independence test works with sequential evaluation (@test-independence_test.R#9) 
  4. Failure: independence test works with multisession evaluation (@test-independence_test.R#19) 
  
  Error: testthat unit tests failed
  Execution halted

Could you please update to check something like:

oplan <- plan()
...
expect_true(identical(plan(), oplan))
@HenrikBengtsson
Copy link
Author

Actually, it should be:

oplan <- plan("list")
...
expect_true(identical(plan(), oplan))

@jfiksel
Copy link
Owner

jfiksel commented Sep 24, 2020

Hi Henrik, thank you for taking the time to let us know about this issue. However, even after installing the version of the future package on your GitHub (version 1.19.1), just running

library(future)
oplan <- plan("list")
identical(plan(), oplan)

returns FALSE, while running

library(future)
oplan <- plan()
identical(plan(), oplan)

returns TRUE. So it seems like your most recent comment would cause an error in the testing script, but let me know if this seems wrong.

@HenrikBengtsson
Copy link
Author

My bad (again), should be:

library(future)
oplan <- plan("list")
identical(plan("list"), oplan)

@jfiksel
Copy link
Owner

jfiksel commented Sep 24, 2020

Excellent, this has been fixed with commit ac0eaa9. Thanks for your help!

@jfiksel jfiksel closed this as completed Sep 24, 2020
@HenrikBengtsson
Copy link
Author

Almost. Right now you're basically testing identical(plan("list"), plan("list")), no different than testing identical(1, 1). You need to record:

oplan <- plan("list")

at the beginning, before you call your functions that set plan() internally. You can test that it works by setting the environment variable R_FUTURE_PLAN=multisession before running your tests.

@jfiksel jfiksel reopened this Sep 24, 2020
@jfiksel
Copy link
Owner

jfiksel commented Sep 24, 2020

Ah, got it. Ok this should now actually be fixed with 8b48c71

@jfiksel jfiksel closed this as completed Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants