diff --git a/NEWS.md b/NEWS.md index 8626eb1d7..3f7a8051e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # usethis (development version) +* `use_zip()` and `use_course()` are equipped to handle a ZIP where the parent folder is implicit (@burnsal, #1961). + * `use_test_helper()` is a new function to create a test helper file (@olivroy, #1822). * `use_cpp11()` makes it easier to update `NAMESPACE` (@pachadotdev, #1921). 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 000000000..ac7c2abd2 Binary files /dev/null and b/tests/testthat/ref/foo-explicit-parent.zip differ diff --git a/tests/testthat/ref/foo-implicit-parent.zip b/tests/testthat/ref/foo-implicit-parent.zip new file mode 100644 index 000000000..0c0a13431 Binary files /dev/null and b/tests/testthat/ref/foo-implicit-parent.zip differ diff --git a/tests/testthat/ref/foo-loose-regular.zip b/tests/testthat/ref/foo-loose-regular.zip deleted file mode 100644 index 92e8cc752..000000000 Binary files a/tests/testthat/ref/foo-loose-regular.zip and /dev/null differ diff --git a/tests/testthat/ref/foo-no-parent.zip b/tests/testthat/ref/foo-no-parent.zip new file mode 100644 index 000000000..11756785f Binary files /dev/null and b/tests/testthat/ref/foo-no-parent.zip differ diff --git a/tests/testthat/ref/foo-not-loose.zip b/tests/testthat/ref/foo-not-loose.zip deleted file mode 100644 index 25c6ee63b..000000000 Binary files a/tests/testthat/ref/foo-not-loose.zip and /dev/null differ diff --git a/tests/testthat/ref/yo-explicit-parent.zip b/tests/testthat/ref/yo-explicit-parent.zip new file mode 100644 index 000000000..5691b075a Binary files /dev/null and b/tests/testthat/ref/yo-explicit-parent.zip differ diff --git a/tests/testthat/ref/yo-implicit-parent.zip b/tests/testthat/ref/yo-implicit-parent.zip new file mode 100644 index 000000000..853ad1ee2 Binary files /dev/null and b/tests/testthat/ref/yo-implicit-parent.zip differ diff --git a/tests/testthat/ref/yo-loose-regular.zip b/tests/testthat/ref/yo-loose-regular.zip deleted file mode 100644 index cbf566165..000000000 Binary files a/tests/testthat/ref/yo-loose-regular.zip and /dev/null differ diff --git a/tests/testthat/ref/yo-no-parent.zip b/tests/testthat/ref/yo-no-parent.zip new file mode 100644 index 000000000..24a7b8565 Binary files /dev/null and b/tests/testthat/ref/yo-no-parent.zip differ diff --git a/tests/testthat/ref/yo-not-loose.zip b/tests/testthat/ref/yo-not-loose.zip deleted file mode 100644 index 5681a0cc7..000000000 Binary files a/tests/testthat/ref/yo-not-loose.zip and /dev/null differ 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/") -})