Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Increase clean testing #185

Closed
MKLau opened this issue Dec 11, 2019 · 2 comments
Closed

Increase clean testing #185

MKLau opened this issue Dec 11, 2019 · 2 comments

Comments

@MKLau
Copy link
Contributor

MKLau commented Dec 11, 2019

From @wlandau:

@MKLau, I like these tests, and I like that they are specific and targeted. I strongly suggest a couple more tests like them for a couple variables in long_script.R.

I also think it is important to compare the actual values of the results you get from actually running the scripts. A cleaned script ultimately needs to produce the same output you get from the original. So for this test, you could

Run source(simple.script, local = some_new_environment) and inspect some_new_environment to get one value of out.

Similarly, get another value of out from the cleaned script.

Compare the two values of out using expect_equal().

@MKLau
Copy link
Contributor Author

MKLau commented Dec 12, 2019

Checking for feedback from ROpenSci reviewers.

@wlandau
Copy link

wlandau commented Dec 13, 2019

Detailed suggestions about https://github.com/MKLau/Rclean/blob/dev/tests/testthat/test-main.R#L54-L97:

  1. What about the ability of cleaned scripts to exclude variables? You could test that fit.xx, fit.sqrt.A, and fit.anova are in env.long but not the other environments.
  2. I think it makes sense to keep all the variables in the same test, but maybe you could condense it down to a loop that calls a function. 3. When you get variables from the environments, get() with inherits = FALSE prevents R from looking in the parent environment for the object, and I think it would add a little safety here.
  3. tempfile()s are more convenient than creating explicitly named scripts and removing them at the end.
  4. withr::with_seed() sets the seed for a chunk of code and restores the original seed afterwards. It could be more convenient and maybe safer.

Sketch:

long_script <- system.file(
  "example",
  "long_script.R",
  package = "Rclean"
)
env_long <- new.env()
withr::with_seed(42, source(long_script, local = env_long))
test_clean <- function(var, long_script, env_long) {
  cleaned <- clean(long_script, var)
  tmp <- tempfile()
  keep(cleaned, tmp)
  env_cleaned <- new.env()
  withr::with_seed(42, source(tmp, local = env_cleaned))
  out <- get(var, env_cleaned, inherits = FALSE)
  exp <- get(var, env_long, inherits = FALSE)
  expect_equal(out, exp)
  expect_true(all(models %in% names(env_long)))
  expect_false(any(models %in% names(env_cleaned)))
  invisible()
}
vars <- list("x", "x2")
tmp <- lapply(
  vars,
  test_clean,
  long_script = long_script,
  env_long = env_long
)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants