moving files during testing is bad

[beanumber]

This line: https://github.com/baumer-lab/fertile/blob/684aff65a02ca88b97997a5e05c96660a2daed33/tests/testthat/test-projects.R#L58

and the corresponding one actually move files around that are tracked by git. This seems very bad, because I have to keep reverting changes just to re-run the tests. Why are these tests moving files around in the testthat directory and not in a temp directory?

I have a proposed solution that I think might also solve some other problems for us: move the test projects to inst and ZIP them. That way they won't ever get modified by tests. When tests want to use the project, they can unzip them to a temp directory.

This would also solve the problem of having nested .Rproj files. What do you think @ambertin ?

[ambertin]

I definitely agree that moving the files in their original directories is bad. I think we originally had them running in a temp directory but if I recall correctly, that was somehow causing other problems (don’t remember the specifics). When I run R CMD check with the current setup, though, it doesn’t actually permanently move the files on my computer so I’m sort of wondering if that’s a Windows thing, since I’m on Mac.

Regardless, I will try playing around with the zipping technique to see if I can get it to work.

[beanumber]

I am using Linux, not Windows.

The behavior of running devtools::test() in the console vs. checking the package seems to be different (another bad sign!).

[ambertin]

Yes I’ve noticed that before. No clue at all how to address it, but considering the tests were working in Travis (before the weird Gert issue), which is testing on a variety of operating systems, it seems to not be a catastrophic issue in practice. Worth looking into for sure, though.

[beanumber]

pkg <- "~/Dropbox/git/baumer-lab/fertile/"
noob <- fs::path(pkg, "tests/testthat/project_noob")
list.files(noob)
#> [1] "project_noob.Rproj" "simple.Rmd"
devtools::test(pkg, filter = "projects")
#> Loading fertile
#> Warning: 
#> ── Conflicts ────────────────────────────────────────────── fertile conflicts ──
#> x read_csv() masks fertile::read_csv()
#> 
#> Did you accidentally source a file rather than using `load_all()`?
#> Run `rm(list = c("read_csv"))` to remove the conflicts.
#> Testing fertile
#> ✓ |  OK F W S | Context
#> ⠏ |   0       | projects-external                                               ⠏ |   0       | projects                                                        ⠏ |   0       | projects-internal                                               ⠏ |   0       | projects                                                        ⠏ |   0       | projects                                                        ⠏ |   0       | projects                                                        ⠋ |   1       | projects                                                        ⠙ |   2       | projects                                                        # A tibble: 1 x 2
#>   culprit   expr                        
#>   <chr>     <glue>                      
#> 1 README.md fs::file_create('README.md')
#> # A tibble: 2 x 2
#>   culprit          expr                            
#>   <fs::path>       <glue>                          
#> 1 ../data/data.csv fs::path_rel('../data/data.csv')
#> 2 ../data/data.csv fs::path_rel('../data/data.csv')
#> 
#> 
#> # A tibble: 1 x 2
#>   culprit   expr                        
#>   <chr>     <glue>                      
#> 1 README.md fs::file_create('README.md')
#> # A tibble: 2 x 2
#>   culprit          expr                            
#>   <fs::path>       <glue>                          
#> 1 ../data/data.csv fs::path_rel('../data/data.csv')
#> 2 ../data/data.csv fs::path_rel('../data/data.csv')
#> â ¼ |   5       | projects                                                        
#> 
#> â ´ |  16       | projects                                                        â § |  18       | projects                                                        â ´ |  26       | projects                                                        x |  29 2     | projects [5.5 s]
#> ────────────────────────────────────────────────────────────────────────────────
#> Failure (test-projects.R:69:3): miceps checking works
#> nrow(dplyr::filter(x$files, ext != "Rproj")) not equal to 8.
#> 1/1 mismatches
#> [1] 15 - 8 == 7
#> 
#> Failure (test-projects.R:74:3): miceps checking works
#> fs::dir_ls(dir, type = "dir") has length 3, not length 0.
#> ────────────────────────────────────────────────────────────────────────────────
#> 
#> ══ Results ═════════════════════════════════════════════════════════════════════
#> Duration: 5.6 s
#> 
#> [ FAIL 2 | WARN 0 | SKIP 0 | PASS 29 ]
list.files(noob)
#> [1] "project_noob.Rproj" "vignettes"

^{Created on 2021-01-03 by the reprex package (v0.3.0)}

[ambertin]

Ah, looks like maybe move files is executing when you run tests in the console, but not when you run it using the "Check" button. Still working on swapping over to zip files (there are a lot of changes that need to be made to the tests) but hopefully that should solve the issue?

[ambertin]

Alright, I've tried using tempdir() but it is causing a whole host of problems for the tests. Going to try another solution where I create a directory temporarily used for testing (that's not a tempdir() and copy the projects in there when proj_move_files() gets tested.

[ambertin]

Addressed in #104.

[beanumber]

@ambertin I fixed some things, but I also broke some things in https://github.com/baumer-lab/fertile/commit/e17f7620ad7dbac31accd64c01ed53d42e053f55.

I don't have time to fix them right now, but can you take a look? Basically all the tests are broken because project_miceps doesn't exist in the tests directory anymore. It should be easy enough to fix by just unzipping the file and putting it in there, but then we have to be careful to destroy it each time. Can you see if you can make that work?

[ambertin]

I'll take a stab at it.

[ambertin]

Addressed the test issues you mentioned plus a problem I found with the edits to sandbox() in #105