From 1f20512433ccfe11821ada166e00b8f8bc0d5d2e Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 26 Jul 2024 15:45:30 -0700 Subject: [PATCH] Refactor tidy_unzip() Fixes #1961, closes #1962. Analyzing the above gave me a new/different understanding of common ZIP file organization structures. I think I've got a better heuristic for setting `exdir`. --- R/course.R | 34 +-- tests/testthat/ref/README.Rmd | 192 +++++++++++--- tests/testthat/ref/README.md | 285 +++++++++++++++------ tests/testthat/ref/foo-explicit-parent.zip | Bin 0 -> 348 bytes tests/testthat/ref/foo-implicit-parent.zip | Bin 0 -> 212 bytes tests/testthat/ref/foo-loose-regular.zip | Bin 204 -> 0 bytes tests/testthat/ref/foo-no-parent.zip | Bin 0 -> 204 bytes tests/testthat/ref/foo-not-loose.zip | Bin 348 -> 0 bytes tests/testthat/ref/yo-explicit-parent.zip | Bin 0 -> 876 bytes tests/testthat/ref/yo-implicit-parent.zip | Bin 0 -> 742 bytes tests/testthat/ref/yo-loose-regular.zip | Bin 718 -> 0 bytes tests/testthat/ref/yo-no-parent.zip | Bin 0 -> 718 bytes tests/testthat/ref/yo-not-loose.zip | Bin 876 -> 0 bytes tests/testthat/test-course.R | 148 ++++++++--- 14 files changed, 494 insertions(+), 165 deletions(-) create mode 100644 tests/testthat/ref/foo-explicit-parent.zip create mode 100644 tests/testthat/ref/foo-implicit-parent.zip delete mode 100644 tests/testthat/ref/foo-loose-regular.zip create mode 100644 tests/testthat/ref/foo-no-parent.zip delete mode 100644 tests/testthat/ref/foo-not-loose.zip create mode 100644 tests/testthat/ref/yo-explicit-parent.zip create mode 100644 tests/testthat/ref/yo-implicit-parent.zip delete mode 100644 tests/testthat/ref/yo-loose-regular.zip create mode 100644 tests/testthat/ref/yo-no-parent.zip delete mode 100644 tests/testthat/ref/yo-not-loose.zip diff --git a/R/course.R b/R/course.R index 772743717..2713568dc 100644 --- a/R/course.R +++ b/R/course.R @@ -360,15 +360,15 @@ tidy_unzip <- function(zipfile, cleanup = FALSE) { ## DropBox ZIP files often include lots of hidden R, RStudio, and Git files filenames <- filenames[keep_lgl(filenames)] - td <- top_directory(filenames) - loose_parts <- is.na(td) - - if (loose_parts) { + parents <- path_before_slash(filenames) + unique_parents <- unique(parents) + if (length(unique_parents) == 1 && unique_parents != "") { + target <- path(base_path, unique_parents) + utils::unzip(zipfile, files = filenames, exdir = base_path) + } else { + # there is no parent; archive contains loose parts target <- path_ext_remove(zipfile) utils::unzip(zipfile, files = filenames, exdir = target) - } else { - target <- path(base_path, td) - utils::unzip(zipfile, files = filenames, exdir = base_path) } ui_bullets(c( "v" = "Unpacking ZIP file into {.path {pth(target, base_path)}} @@ -398,7 +398,7 @@ tidy_unzip <- function(zipfile, cleanup = FALSE) { } } - invisible(target) + invisible(unclass(target)) } #' @rdname use_course_details @@ -525,15 +525,17 @@ keep_lgl <- function(file, !grepl(ignores, file, perl = TRUE) } -top_directory <- function(filenames) { - in_top <- path_dir(filenames) == "." - unique_top <- unique(filenames[in_top]) - is_directory <- grepl("/$", unique_top) - if (length(unique_top) > 1 || !is_directory) { - NA_character_ - } else { - unique_top +path_before_slash <- function(filepath) { + f <- function(x) { + parts <- strsplit(x, "/", fixed = TRUE)[[1]] + if (length(parts) > 1 || grepl("/", x)) { + parts[1] + } else { + "" + } } + purrr::map_chr(filepath, f) + } content_type <- function(h) { diff --git a/tests/testthat/ref/README.Rmd b/tests/testthat/ref/README.Rmd index d02b4e6d4..d95bafab6 100644 --- a/tests/testthat/ref/README.Rmd +++ b/tests/testthat/ref/README.Rmd @@ -14,66 +14,169 @@ library(fs) ## Different styles of ZIP file -Examples based on foo folder found here. +usethis has an unexported function `tidy_unzip()`, which is used under the hood in `use_course()` and `use_zip()`. +It is a wrapper around `utils::unzip()` that uses some heuristics to choose a good value for `exdir`, which is the "the directory to extract files to." +Why do we do this? +Because it's really easy to _not_ get the desired result when unpacking a ZIP archive. + +Common aggravations: + +* Instead of the unpacked files being corraled within a folder, they explode as "loose parts" into the current working directory. Too little nesting. +* The unpacked files are contained in a folder, but that folder itself is contained inside another folder. Too much nesting. + +`tidy_unzip()` tries to get the nesting just right. + +Why doesn't unzipping "just work"? +Because the people who make `.zip` files make lots of different choices when they actually create the archive and these details aren't baked in, i.e. a successful roundtrip isn't automatic. +It usually requires some peeking inside the archive and adjusting the unpack options. + +This README documents specific `.zip` situations that we anticipate. + +## Explicit parent folder + +Consider the foo folder: ```{bash} tree foo ``` -### Not Loose Parts, a.k.a. GitHub style +Zip it up like so: + +```{bash, eval = FALSE} +zip -r foo-explicit-parent.zip foo/ +``` + +This is the type of ZIP file that we get from GitHub via links of the forms and . + +Inspect it in the shell: + +```{bash} +unzip -Z1 foo-explicit-parent.zip +``` + +Or from R: + +```{r} +foo_files <- unzip("foo-explicit-parent.zip", list = TRUE) +with( + foo_files, + data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) +) +``` + +Note that the folder `foo/` is explicitly included and all of the files are contained in it (in this case, just one file). + +## Implicit parent folder + +Consider the foo folder: + +```{bash} +tree foo +``` -This is the structure of ZIP files yielded by GitHub via links of the forms and . +Zip it up like so: ```{bash, eval = FALSE} -zip -r foo-not-loose.zip foo/ +zip -r foo-implicit-parent.zip foo/* ``` -Notice that everything is packaged below one top-level directory. +Note the use of `foo/*`, as opposed to `foo` or `foo/`. +This type of ZIP file was reported in . +The example given there is . + +Inspect our small example in the shell: + +```{bash} +unzip -Z1 foo-implicit-parent.zip +``` + +Or from R: ```{r} -foo_not_loose_files <- unzip("foo-not-loose.zip", list = TRUE) +foo_files <- unzip("foo-implicit-parent.zip", list = TRUE) with( - foo_not_loose_files, + foo_files, data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) ) ``` -### Loose Parts, the Regular Way +Note that `foo/` is not included and its (original) existence is just implicit in the relative path to, e.g., +`foo/file.txt`. + +Here's a similar look at the example from issue #1961: + +```bash +~/rrr/usethis/tests/testthat/ref % unzip -l ~/Downloads/Species\ v2.3.zip +Archive: /Users/jenny/Downloads/Species v2.3.zip + Length Date Time Name +--------- ---------- ----- ---- + 1241 04-27-2023 09:16 species_v2/label_encoder.txt +175187560 04-06-2023 15:13 species_v2/model_arch.pt +174953575 04-14-2023 12:28 species_v2/model_weights.pth +--------- ------- +350142376 3 files +``` + +Note also that the implicit parent folder `species_v2` is not the base of the ZIP file name `Species v2.3.zip`. -This is the structure of many ZIP files I've seen, just in general. +## No parent + +Consider the foo folder: + +```{bash} +tree foo +``` + +Zip it up like so: ```{bash, eval = FALSE} -cd foo -zip ../foo-loose-regular.zip * -cd .. +(cd foo && zip -r ../foo-no-parent.zip .) ``` -All the files are packaged in the ZIP archive as "loose parts", i.e. there is no explicit top-level directory. +Note that we are zipping everything in a folder from *inside* the folder. + +Inspect our small example in the shell: + +```{bash} +unzip -Z1 foo-no-parent.zip +``` + +Or from R: ```{r} -foo_loose_regular_files <- unzip("foo-loose-regular.zip", list = TRUE) +foo_files <- unzip("foo-no-parent.zip", list = TRUE) with( - foo_loose_regular_files, + foo_files, data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) ) ``` -### Loose Parts, the DropBox Way +All the files are packaged in the ZIP archive as "loose parts", i.e. there is no explicit or implicit top-level directory. + +## No parent, the DropBox Variation This is the structure of ZIP files yielded by DropBox via links of this form . I can't figure out how to even do this with zip locally, so I had to create an example on DropBox and download it. Jim Hester reports it is possible with `archive::archive_write_files()`. -It's basically like the "loose parts" above, except it includes a spurious top-level directory `"/"`. +It's basically like the "no parent" example above, except it includes a spurious top-level directory `"/"`. + +Inspect our small example in the shell: + +```{bash} +unzip -Z1 foo-loose-dropbox.zip +``` + +Or from R: ```{r} # curl::curl_download( # "https://www.dropbox.com/sh/5qfvssimxf2ja58/AABz3zrpf-iPYgvQCgyjCVdKa?dl=1", # destfile = "foo-loose-dropbox.zip" # ) -foo_loose_dropbox_files <- unzip("foo-loose-dropbox.zip", list = TRUE) +foo_files <- unzip("foo-loose-dropbox.zip", list = TRUE) with( - foo_loose_dropbox_files, + foo_files, data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) ) ``` @@ -87,43 +190,50 @@ mapname: conversion of failed inflating: file.txt ``` -So this is a pretty odd ZIP packing strategy. But we need to plan for it. +which indicates some tripping over the `/`. +Only `file.txt` is left behind. -## Subdirs only at top-level +This is a pretty odd ZIP packing strategy. But we need to plan for it. -Let's make sure we detect loose parts (or not) when the top-level has only directories, not files. +## Only subdirectories -Example based on the yo directory here: +For testing purposes, we also want an example where all the files are inside subdirectories. + +Examples based on the yo directory here: ```{bash} tree yo ``` -```{bash, eval = FALSE} -zip -r yo-not-loose.zip yo/ -``` - -```{r} -(yo_not_loose_files <- unzip("yo-not-loose.zip", list = TRUE)) -top_directory(yo_not_loose_files$Name) -``` +Zip it up, in all the usual ways: ```{bash, eval = FALSE} -cd yo -zip -r ../yo-loose-regular.zip * -cd .. +zip -r yo-explicit-parent.zip yo/ +zip -r yo-implicit-parent.zip yo/* +(cd yo && zip -r ../yo-no-parent.zip .) ``` -```{r} -(yo_loose_regular_files <- unzip("yo-loose-regular.zip", list = TRUE)) -top_directory(yo_loose_regular_files$Name) -``` +Again, I couldn't create the DropBox variant locally, so I did it by downloading from DropBox. -```{r} +```{r eval = FALSE} # curl::curl_download( # "https://www.dropbox.com/sh/afydxe6pkpz8v6m/AADHbMZAaW3IQ8zppH9mjNsga?dl=1", # destfile = "yo-loose-dropbox.zip" # ) -(yo_loose_dropbox_files <- unzip("yo-loose-dropbox.zip", list = TRUE)) -top_directory(yo_loose_dropbox_files$Name) +``` + +Inspect each in the shell: + +```{bash} +unzip -Z1 yo-explicit-parent.zip +``` + +```{bash} +unzip -Z1 yo-implicit-parent.zip +``` +```{bash} +unzip -Z1 yo-no-parent.zip +``` +```{bash} +unzip -Z1 yo-loose-dropbox.zip ``` diff --git a/tests/testthat/ref/README.md b/tests/testthat/ref/README.md index cc301f187..b25b9c00e 100644 --- a/tests/testthat/ref/README.md +++ b/tests/testthat/ref/README.md @@ -4,39 +4,72 @@ ZIP file structures ``` r devtools::load_all("~/rrr/usethis") #> ℹ Loading usethis -#> x unloadNamespace("usethis") failed because another loaded package needs it -#> ℹ Forcing unload. If you encounter problems, please restart R. library(fs) ``` ## Different styles of ZIP file -Examples based on foo folder found here. +usethis has an unexported function `tidy_unzip()`, which is used under +the hood in `use_course()` and `use_zip()`. It is a wrapper around +`utils::unzip()` that uses some heuristics to choose a good value for +`exdir`, which is the “the directory to extract files to.” Why do we do +this? Because it’s really easy to *not* get the desired result when +unpacking a ZIP archive. + +Common aggravations: + +- Instead of the unpacked files being corraled within a folder, they + explode as “loose parts” into the current working directory. Too + little nesting. +- The unpacked files are contained in a folder, but that folder itself + is contained inside another folder. Too much nesting. + +`tidy_unzip()` tries to get the nesting just right. + +Why doesn’t unzipping “just work”? Because the people who make `.zip` +files make lots of different choices when they actually create the +archive and these details aren’t baked in, i.e. a successful roundtrip +isn’t automatic. It usually requires some peeking inside the archive and +adjusting the unpack options. + +This README documents specific `.zip` situations that we anticipate. + +## Explicit parent folder + +Consider the foo folder: ``` bash tree foo -#> foo +#> foo #> └── file.txt -#> -#> 0 directories, 1 file +#> +#> 1 directory, 1 file +``` + +Zip it up like so: + +``` bash +zip -r foo-explicit-parent.zip foo/ ``` -### Not Loose Parts, a.k.a. GitHub style +This is the type of ZIP file that we get from GitHub via links of the +forms and +. -This is the structure of ZIP files yielded by GitHub via links of the -forms and -. +Inspect it in the shell: ``` bash -zip -r foo-not-loose.zip foo/ +unzip -Z1 foo-explicit-parent.zip +#> foo/ +#> foo/file.txt ``` -Notice that everything is packaged below one top-level directory. +Or from R: ``` r -foo_not_loose_files <- unzip("foo-not-loose.zip", list = TRUE) +foo_files <- unzip("foo-explicit-parent.zip", list = TRUE) with( - foo_not_loose_files, + foo_files, data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) ) #> Name dirname basename @@ -44,30 +77,115 @@ with( #> 2 foo/file.txt foo file.txt ``` -### Loose Parts, the Regular Way +Note that the folder `foo/` is explicitly included and all of the files +are contained in it (in this case, just one file). + +## Implicit parent folder -This is the structure of many ZIP files I’ve seen, just in general. +Consider the foo folder: ``` bash -cd foo -zip ../foo-loose-regular.zip * -cd .. +tree foo +#> foo +#> └── file.txt +#> +#> 1 directory, 1 file ``` -All the files are packaged in the ZIP archive as “loose parts”, -i.e. there is no explicit top-level directory. +Zip it up like so: + +``` bash +zip -r foo-implicit-parent.zip foo/* +``` + +Note the use of `foo/*`, as opposed to `foo` or `foo/`. This type of ZIP +file was reported in . The +example given there is +. + +Inspect our small example in the shell: + +``` bash +unzip -Z1 foo-implicit-parent.zip +#> foo/file.txt +``` + +Or from R: + +``` r +foo_files <- unzip("foo-implicit-parent.zip", list = TRUE) +with( + foo_files, + data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) +) +#> Name dirname basename +#> 1 foo/file.txt foo file.txt +``` + +Note that `foo/` is not included and its (original) existence is just +implicit in the relative path to, e.g., `foo/file.txt`. + +Here’s a similar look at the example from issue \#1961: + +``` bash +~/rrr/usethis/tests/testthat/ref % unzip -l ~/Downloads/Species\ v2.3.zip +Archive: /Users/jenny/Downloads/Species v2.3.zip + Length Date Time Name +--------- ---------- ----- ---- + 1241 04-27-2023 09:16 species_v2/label_encoder.txt +175187560 04-06-2023 15:13 species_v2/model_arch.pt +174953575 04-14-2023 12:28 species_v2/model_weights.pth +--------- ------- +350142376 3 files +``` + +Note also that the implicit parent folder `species_v2` is not the base +of the ZIP file name `Species v2.3.zip`. + +## No parent + +Consider the foo folder: + +``` bash +tree foo +#> foo +#> └── file.txt +#> +#> 1 directory, 1 file +``` + +Zip it up like so: + +``` bash +(cd foo && zip -r ../foo-no-parent.zip .) +``` + +Note that we are zipping everything in a folder from *inside* the +folder. + +Inspect our small example in the shell: + +``` bash +unzip -Z1 foo-no-parent.zip +#> file.txt +``` + +Or from R: ``` r -foo_loose_regular_files <- unzip("foo-loose-regular.zip", list = TRUE) +foo_files <- unzip("foo-no-parent.zip", list = TRUE) with( - foo_loose_regular_files, + foo_files, data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) ) #> Name dirname basename #> 1 file.txt . file.txt ``` -### Loose Parts, the DropBox Way +All the files are packaged in the ZIP archive as “loose parts”, +i.e. there is no explicit or implicit top-level directory. + +## No parent, the DropBox Variation This is the structure of ZIP files yielded by DropBox via links of this form . I can’t @@ -77,21 +195,31 @@ with `archive::archive_write_files()`. -It’s basically like the “loose parts” above, except it includes a +It’s basically like the “no parent” example above, except it includes a spurious top-level directory `"/"`. +Inspect our small example in the shell: + +``` bash +unzip -Z1 foo-loose-dropbox.zip +#> / +#> file.txt +``` + +Or from R: + ``` r # curl::curl_download( # "https://www.dropbox.com/sh/5qfvssimxf2ja58/AABz3zrpf-iPYgvQCgyjCVdKa?dl=1", # destfile = "foo-loose-dropbox.zip" # ) -foo_loose_dropbox_files <- unzip("foo-loose-dropbox.zip", list = TRUE) +foo_files <- unzip("foo-loose-dropbox.zip", list = TRUE) with( - foo_loose_dropbox_files, + foo_files, data.frame(Name = Name, dirname = path_dir(Name), basename = path_file(Name)) ) #> Name dirname basename -#> 1 / / +#> 1 / / #> 2 file.txt . file.txt ``` @@ -103,72 +231,79 @@ result: mapname: conversion of failed inflating: file.txt -So this is a pretty odd ZIP packing strategy. But we need to plan for -it. +which indicates some tripping over the `/`. Only `file.txt` is left +behind. + +This is a pretty odd ZIP packing strategy. But we need to plan for it. -## Subdirs only at top-level +## Only subdirectories -Let’s make sure we detect loose parts (or not) when the top-level has -only directories, not files. +For testing purposes, we also want an example where all the files are +inside subdirectories. -Example based on the yo directory here: +Examples based on the yo directory here: ``` bash tree yo -#> yo -#> ├── subdir1 +#> yo +#> ├── subdir1 #> │   └── file1.txt -#> └── subdir2 +#> └── subdir2 #> └── file2.txt -#> -#> 2 directories, 2 files +#> +#> 3 directories, 2 files ``` +Zip it up, in all the usual ways: + ``` bash -zip -r yo-not-loose.zip yo/ +zip -r yo-explicit-parent.zip yo/ +zip -r yo-implicit-parent.zip yo/* +(cd yo && zip -r ../yo-no-parent.zip .) ``` +Again, I couldn’t create the DropBox variant locally, so I did it by +downloading from DropBox. + ``` r -(yo_not_loose_files <- unzip("yo-not-loose.zip", list = TRUE)) -#> Name Length Date -#> 1 yo/ 0 2018-01-11 15:48:00 -#> 2 yo/subdir1/ 0 2018-01-11 15:48:00 -#> 3 yo/subdir1/file1.txt 42 2018-01-11 15:48:00 -#> 4 yo/subdir2/ 0 2018-01-11 15:49:00 -#> 5 yo/subdir2/file2.txt 42 2018-01-11 15:49:00 -top_directory(yo_not_loose_files$Name) -#> [1] "yo/" +# curl::curl_download( +# "https://www.dropbox.com/sh/afydxe6pkpz8v6m/AADHbMZAaW3IQ8zppH9mjNsga?dl=1", +# destfile = "yo-loose-dropbox.zip" +# ) ``` +Inspect each in the shell: + ``` bash -cd yo -zip -r ../yo-loose-regular.zip * -cd .. +unzip -Z1 yo-explicit-parent.zip +#> yo/ +#> yo/subdir2/ +#> yo/subdir2/file2.txt +#> yo/subdir1/ +#> yo/subdir1/file1.txt ``` -``` r -(yo_loose_regular_files <- unzip("yo-loose-regular.zip", list = TRUE)) -#> Name Length Date -#> 1 subdir1/ 0 2018-01-11 15:48:00 -#> 2 subdir1/file1.txt 42 2018-01-11 15:48:00 -#> 3 subdir2/ 0 2018-01-11 15:49:00 -#> 4 subdir2/file2.txt 42 2018-01-11 15:49:00 -top_directory(yo_loose_regular_files$Name) -#> [1] NA +``` bash +unzip -Z1 yo-implicit-parent.zip +#> yo/subdir1/ +#> yo/subdir1/file1.txt +#> yo/subdir2/ +#> yo/subdir2/file2.txt ``` -``` r -# curl::curl_download( -# "https://www.dropbox.com/sh/afydxe6pkpz8v6m/AADHbMZAaW3IQ8zppH9mjNsga?dl=1", -# destfile = "yo-loose-dropbox.zip" -# ) -(yo_loose_dropbox_files <- unzip("yo-loose-dropbox.zip", list = TRUE)) -#> Name Length Date -#> 1 / 0 2018-01-11 23:57:00 -#> 2 subdir1/file1.txt 42 2018-01-11 23:57:00 -#> 3 subdir2/file2.txt 42 2018-01-11 23:57:00 -#> 4 subdir1/ 0 2018-01-11 23:57:00 -#> 5 subdir2/ 0 2018-01-11 23:57:00 -top_directory(yo_loose_dropbox_files$Name) -#> [1] NA +``` bash +unzip -Z1 yo-no-parent.zip +#> subdir2/ +#> subdir2/file2.txt +#> subdir1/ +#> subdir1/file1.txt +``` + +``` bash +unzip -Z1 yo-loose-dropbox.zip +#> / +#> subdir1/file1.txt +#> subdir2/file2.txt +#> subdir1/ +#> subdir2/ ``` diff --git a/tests/testthat/ref/foo-explicit-parent.zip b/tests/testthat/ref/foo-explicit-parent.zip new file mode 100644 index 0000000000000000000000000000000000000000..ac7c2abd21ae7dd00e7c00d8cd16cf3dc3a067b1 GIT binary patch literal 348 zcmWIWW@h1H00HTYUlCvilwe_yVMxo**AEThWMD33Sds?D72FJrEZ-Oz7+6Gr>H<)7 zi@g0?uLcwaVIHXNw9K4Ty^@L&xG5DNlTc0ZR7lKKfGJcc&&W*9P{_$FOD$H&%qz}J zNmT$jgDb$Bkx7mjms=&Et`=Z;>j+{(T+Rx0IjVi=CZW0;ViM4Y3`-jApeDf_7aD@g SL{>JCtxP~T2S~31aToyjgF-+6 literal 0 HcmV?d00001 diff --git a/tests/testthat/ref/foo-implicit-parent.zip b/tests/testthat/ref/foo-implicit-parent.zip new file mode 100644 index 0000000000000000000000000000000000000000..0c0a13431c26d6f286faf8edeb9e77b7ab6cdc3c GIT binary patch literal 212 zcmWIWW@h1H00HTYUlAg2|JJJk*&xirAj6QBpRb>mnUktlQc)5b!pXo~%CID@0*Fg1 zxEUB(zA-W|u!sQFdMYI5D!>#flxJinXDH-kmZcUeWabrTrlcx>4B`s#W@M6M#$}rX m&>RK^pot7i8bK^nyICQ2quCqa&B_K+%Ls%)KspD+VE_O?Q!V}g literal 0 HcmV?d00001 diff --git a/tests/testthat/ref/foo-loose-regular.zip b/tests/testthat/ref/foo-loose-regular.zip deleted file mode 100644 index 92e8cc7529dd67b4b19f0aadd1e6dae4ed1d037c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 204 zcmWIWW@h1H00Fi@Z6A@hf9utNY!K#PkYPy6%t_TNsVE5z;bdUuUJ)KuyDL1Zw1S&~ zk>x8R0|Sc)P@ShjVy*&Ap+b2^W^#r?PG(tZu|j5Eab`-YLRx;lK39M@Ba<96E}JBP n<}fe-O=MWo2x7sE3k_j~*o$UofHx}}NF5^(`T^+-5QhN(Pp~N{ diff --git a/tests/testthat/ref/foo-no-parent.zip b/tests/testthat/ref/foo-no-parent.zip new file mode 100644 index 0000000000000000000000000000000000000000..11756785f9043874a5e0a826857392d7079e8fd5 GIT binary patch literal 204 zcmWIWW@h1H00HTYUlAg2|JJJk*&xipAj6QBnUktlQc)5b!pXo~%CID@0*Fg1xEUB( zzA-W|u!sQFc`794D!>#flxJinXDH-kmZcUeWabrTrlcyQ<>%{j1$Z+u$uZ-yNdjmN l0|U@Rh9!+47R11vdjD%U4DQ1{M*Z zx&XLtHlS{iw}0!^fTAGG1J#|DnUktlQc(gng?mMK)GDA!Xr_27B<3o>6e^TwWF}`Q zO-jiuG9tl{Z8y%OXaS7|VG&eA(lT>Wjr2+?O5jea067cQNuCOcxe9QVItn@Y z$%!SYDGEucIr-%ZK>xs;#6_gz4M}kPuj_Mdu{z#RALJxMq8$%biPQ0h`as7sGRZOH ziZ}^q6bdlBbp$cN(Z~voM%3`aXAn07vO%^GgJ9to8bX|bs1XV=5SV@#mNYh@8VHXx zVoe5WhXYin{sM}_qZ$-}#5ol;`jDM!4|Fk-fk=^w%VbtIklR>+a6VA)c94%50IXfF AYybcN literal 0 HcmV?d00001 diff --git a/tests/testthat/ref/yo-implicit-parent.zip b/tests/testthat/ref/yo-implicit-parent.zip new file mode 100644 index 0000000000000000000000000000000000000000..853ad1ee220e33da86b6a5e1c17936f9cdbfe7cf GIT binary patch literal 742 zcmWIWW@h1H00HTYUlCvil;CENVW`a4FD^|=$t*I|4-MgDU@m1?k_N^V+zgB?-xwJf zSVVxz15gb4b$za_7Ely~MNkb%%gjkN)GMhdftywVG7;4@Pld!>1-MEbg`E84#FEq$ zg{0J+{Bi{(CvlW7AKGBDrx9vQ zesW?-YKlTqYEFK+LS?=_%spHPOBHcC+X&+9cZ8g+0yJUUjWa2j&Nk8qImQU?>@NhI z4OWTM*+%+6XEQR%G2;qv324X*FuZjHF=4(94Pk|ZK1RqROv4O&h-tuBVp!7Xgk%~# ze8I+oVgiq`n4yd8$n9`rL17JaBq*%$n1&hB$d3GpWExVq1C0fRJBG2WY(Vu4EI{Z5 K^kg2$qYMC#ZI^KX diff --git a/tests/testthat/ref/yo-no-parent.zip b/tests/testthat/ref/yo-no-parent.zip new file mode 100644 index 0000000000000000000000000000000000000000..24a7b85656d9b31e97a14c6b54de8985f779e0c6 GIT binary patch literal 718 zcmWIWW@h1H00HTYUlCvil;B{HVJI$5O35rT(hm*cWMD33Sds?D72FJrEZ-Oz7+6Gr zY6DPA*mmPgiWX25gawgINXyJgHPS1oD1jSQ0WuENC{KmNTm`sF9fh3y?7g}N9_A)IYUg0p{JpKFWhY(ssJV+@IQHdrN2XB+ARoz2K3$BZky zC7}KgV0h~YV#0hI8o~+p3DPzlmP&Ue3LZ* literal 0 HcmV?d00001 diff --git a/tests/testthat/ref/yo-not-loose.zip b/tests/testthat/ref/yo-not-loose.zip deleted file mode 100644 index 5681a0cc71d4292ed518666cc49a62df7109a441..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 876 zcmWIWW@h1H00GH5Z67cLN-#6XFjVI2hlX%6FhBYp9@Y3GJgT&Un}Lz#D;Onr3c@=Hk+%l*}SSh#@x!8NvlLEF7Re-D1QOLkF2{pa;&i-`KG5-uOmfV)B2EGtg#rw39YIWRG_pdX5zR_u zgD@iz*&thpL9p-(4FMVmic&lVVn!&$Kw$b|Skl;pY9Kt)z$SyD7mvx9QHJc)U&tnd wA`s|QPz2&J5HtFaooWwsF+A;{L?+N=ED;&t&B_LJ0|P4%&Ijt<4)QSr0KiMO0{{R3 diff --git a/tests/testthat/test-course.R b/tests/testthat/test-course.R index 0dff96f88..987a64fbb 100644 --- a/tests/testthat/test-course.R +++ b/tests/testthat/test-course.R @@ -83,27 +83,121 @@ test_that("tidy_download() works", { ## tidy_unzip ---- -test_that("tidy_unzip() deals with loose parts, reports unpack destination", { - tmp <- file_temp(ext = ".zip") - fs::file_copy(test_file("yo-loose-regular.zip"), tmp) - dest <- tidy_unzip(tmp) - loose_regular_files <- fs::path_file(fs::dir_ls(dest, recurse = TRUE)) - fs::dir_delete(dest) - - tmp <- file_temp(ext = ".zip") - fs::file_copy(test_file("yo-loose-dropbox.zip"), tmp) - dest <- tidy_unzip(tmp) - loose_dropbox_files <- fs::path_file(fs::dir_ls(dest, recurse = TRUE)) - fs::dir_delete(dest) - - tmp <- file_temp(ext = ".zip") - fs::file_copy(test_file("yo-not-loose.zip"), tmp) - dest <- tidy_unzip(tmp) - not_loose_files <- fs::path_file(fs::dir_ls(dest, recurse = TRUE)) - fs::dir_delete(dest) - - expect_identical(loose_regular_files, loose_dropbox_files) - expect_identical(loose_dropbox_files, not_loose_files) +test_that("tidy_unzip(): explicit parent, file example", { + local_interactive(FALSE) + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("foo-explicit-parent.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), "foo") + + explicit_parent_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_equal(explicit_parent_files, "file.txt") +}) + +test_that("tidy_unzip(): explicit parent, folders example", { + local_interactive(FALSE) + files <- c("subdir1", "file1.txt", "subdir2", "file2.txt") + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("yo-explicit-parent.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), "yo") + + explicit_parent_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_setequal(explicit_parent_files, files) +}) + +test_that("tidy_unzip(): implicit parent, file example", { + local_interactive(FALSE) + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("foo-implicit-parent.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), "foo") + + implicit_parent_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_equal(implicit_parent_files, "file.txt") +}) + +test_that("tidy_unzip(): implicit parent, folders example", { + local_interactive(FALSE) + files <- c("subdir1", "file1.txt", "subdir2", "file2.txt") + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("yo-implicit-parent.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), "yo") + + implicit_parent_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_setequal(implicit_parent_files, files) +}) + +test_that("tidy_unzip(): no parent, file example", { + local_interactive(FALSE) + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("foo-no-parent.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), path_ext_remove(path_file(zipfile))) + + no_parent_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_setequal(no_parent_files, "file.txt") +}) + +test_that("tidy_unzip(): no parent, folders example", { + local_interactive(FALSE) + files <- c("subdir1", "file1.txt", "subdir2", "file2.txt") + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("yo-no-parent.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), path_ext_remove(path_file(zipfile))) + + no_parent_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_setequal(no_parent_files, files) +}) + +test_that("tidy_unzip(): DropBox, file example", { + local_interactive(FALSE) + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("foo-loose-dropbox.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), path_ext_remove(path_file(zipfile))) + + loose_dropbox_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_setequal(loose_dropbox_files, "file.txt") +}) + +test_that("tidy_unzip(): DropBox, folders example", { + local_interactive(FALSE) + files <- c("subdir1", "file1.txt", "subdir2", "file2.txt") + + zipfile <- withr::local_tempfile(fileext = ".zip") + file_copy(test_file("yo-loose-dropbox.zip"), zipfile) + dest <- tidy_unzip(zipfile) + withr::defer(dir_delete(dest)) + expect_equal(path_file(dest), path_ext_remove(path_file(zipfile))) + + loose_dropbox_files <- path_file(dir_ls(dest, recurse = TRUE)) + expect_setequal(loose_dropbox_files, files) +}) + +test_that("path_before_slash() works", { + expect_equal(path_before_slash(""), "") + expect_equal(path_before_slash("/"), "") + expect_equal(path_before_slash("a/"), "a") + expect_equal(path_before_slash("a/b"), "a") + expect_equal(path_before_slash("a/b/c"), "a") + expect_equal(path_before_slash("a/b/c/"), "a") }) ## helpers ---- @@ -260,15 +354,3 @@ test_that("keep_lgl() keeps and drops correct files", { ) expect_false(any(keep_lgl(droppers))) }) - -test_that("top_directory() identifies a unique top directory (or not)", { - ## there is >= 1 file at top-level or >1 directories - expect_identical(top_directory("a"), NA_character_) - expect_identical(top_directory(c("a/", "b")), NA_character_) - expect_identical(top_directory(c("a/", "b/")), NA_character_) - - ## there are no files at top-level and exactly 1 directory - expect_identical(top_directory("a/"), "a/") - expect_identical(top_directory(c("a/", "a/b")), "a/") - expect_identical(top_directory(c("a/", "a/b", "a/c")), "a/") -})