improve solution

Polkas · Sep 23, 2024 · 78d9cbc · 78d9cbc
1 parent b714ac9
commit 78d9cbc
Show file tree

Hide file tree

Showing 19 changed files with 118 additions and 68 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,9 +1,9 @@
 # pacs 0.5.1.9000
 
-* add NEWS file related functions, pac_news and pac_compare_news.
-* improve code base.
+* add new functions: pac_news and pac_compare_news. Functions are NEWS file related.
 * update the tinyverse vignette with a new badge url.
 * fix a problem with app_deps on R 3.6.
+* improve code base.
 
 # pacs 0.5.1
 

diff --git a/R/comapre.R b/R/comapre.R
@@ -194,6 +194,10 @@ pac_compare_news <- function(pac,
 
   stopifnot(utils::compareVersion(new, old) >= 0)
 
+  if (utils::compareVersion(new, old) == 0) {
+    return(NA)
+  }
+
   version_pattern <- function(version) {
       paste0("#.*", version)
   }
@@ -204,15 +208,15 @@ pac_compare_news <- function(pac,
 
   old_version_reg <- regexpr(version_pattern(old), pac_news)
   which_matched_old <- which(old_version_reg > 0)[1]
-  old_version_pos <- if (which_matched_old > 0) {
+  old_version_pos <- if (isTRUE(which_matched_old > 0)) {
     which_matched_old
   } else {
     NA
   }
 
   new_version_reg <- regexpr(version_pattern(new), pac_news)
   which_matched_new <- which(new_version_reg > 0)[1]
-  new_version_pos <- if (which_matched_new > 0) {
+  new_version_pos <- if (isTRUE(which_matched_new > 0)) {
     which_matched_new
   } else {
     NA
@@ -222,7 +226,7 @@ pac_compare_news <- function(pac,
     return(NA)
   }
 
-  result <- pac_news[new_version_pos:(old_version_pos - 1)]
+  result <- pac_news[new_version_pos:max(c(old_version_pos - 1, 1))]
   result
 }
 

diff --git a/R/description.R b/R/description.R
@@ -78,12 +78,11 @@ pac_description_dcf_raw <- function(pac, version, repos = "https://cran.rstudio.
     silent = TRUE
   )
   if (inherits(tt, "try-error")) {
-    result <- cran_archive_file(pac, version, "DESCRIPTION", repos)
+    result <- read_cran_file(pac, version, "DESCRIPTION", repos)
   } else {
     result <- as.list(read.dcf(ee)[1, ])
   }
   unlink(ee)
-
   structure(result, package = pac, version = version)
 }
 

diff --git a/R/namespace.R b/R/namespace.R
@@ -80,7 +80,7 @@ pac_readnamespace_raw <- function(pac, version, repos = "https://cran.rstudio.co
     silent = TRUE
   )
   if (inherits(tt, "try-error")) {
-    result <- cran_archive_file(pac, version, "NAMESPACE", repos)
+    result <- read_cran_file(pac, version, "NAMESPACE", repos)
   } else {
     result <- readLines(ee, warn = FALSE)
     unlink(ee)

diff --git a/R/news.R b/R/news.R
@@ -83,11 +83,12 @@ pac_readnews_raw <- function(pac, version, repos = "https://cran.rstudio.com/")
     silent = TRUE
   )
   if (inherits(tt, "try-error")) {
-    result <- cran_archive_file(pac, version, "NEWS", repos)
+    result <- read_cran_file(pac, version, "NEWS", repos)
   } else {
-    res <- readLines(ee)
+    result <- readLines(ee)
     unlink(ee)
   }
+  result
 }
 
 pac_readnews <- memoise::memoise(pac_readnews_raw, cache = cachem::cache_mem(max_age = 30 * 60))
diff --git a/R/read_file_cran.R → R/read_cran_file.R b/R/read_file_cran.R → R/read_cran_file.R
@@ -1,11 +1,10 @@
 #' Read a file from CRAN
 #' @description Read a file from CRAN package source.
-#' @param pac `character` package name.
-#' @param version `character` package version.
-#' @param repos `character` vector repositories URLs to use. Used only for the validation. Default `https://cran.rstudio.com/`
-#' @param file `character` file name to read. Possible values are `DESCRIPTION` and `NAMESPACE`.
+#' @inheritParams standard_args
 #' @keywords internal
-cran_archive_file <- function(pac, version, file, repos = "https://cran.rstudio.com/") {
+read_cran_file <- function(pac, version, file, repos = "https://cran.rstudio.com/") {
+  stopifnot(is_online())
+
   last_version <- pac_last(pac, repos)
 
   if (isTRUE(!is.null(version) && version != last_version)) {
@@ -35,7 +34,7 @@ cran_archive_file <- function(pac, version, file, repos = "https://cran.rstudio.
   )
 
   if (inherits(download, "try-error")) {
-    result <- structure(list(), package = pac, version = version)
+    result <- NA
   } else {
     temp_dir <- tempdir()
     utils::untar(temp_tar, exdir = temp_dir)

diff --git a/R/read_file_github.R → R/read_github_file.R b/R/read_file_github.R → R/read_github_file.R
@@ -1,12 +1,10 @@
 #' Read a file from a GitHub CRAN repository
 #' @description Read a file from a GitHub CRAN repository.
-#' @param pac `character` package name.
-#' @param version `character` package version.
-#' @param file `character` file name to read. Possible values are `DESCRIPTION` and `NAMESPACE`.
-#' @param repos `character` vector repositories URLs to use. Used only for the validation. Default `https://cran.rstudio.com/`
+#' @inheritParams standard_args
 #' @note if the file is not found in the GitHub repository, it will try to find it in the CRAN archive.
 #' @keywords internal
 read_github_file <- function(pac, version, file, repos = "https://cran.rstudio.com/") {
+  stopifnot(is_online())
   ee <- tempfile()
   d_url <- sprintf(
     "https://raw.githubusercontent.com/cran/%s/%s/%s",
@@ -24,9 +22,10 @@ read_github_file <- function(pac, version, file, repos = "https://cran.rstudio.c
     silent = TRUE
   )
   if (inherits(tt, "try-error")) {
-    result <- cran_archive_file(pac, version, repos, file)
+    result <- read_cran_file(pac, version, file, repos)
   } else {
-    res <- readLines(ee)
+    result <- readLines(ee)
     unlink(ee)
   }
+  result
 }
diff --git a/R/template_args.R b/R/template_args.R
@@ -12,6 +12,7 @@
 #' Default: `list(scope = character(0), flavor = NULL)`
 #' @param description_v `logical` if the dependencies version should be taken from description files, minimal required. By default installed versions are taken. Default: `FALSE`
 #' @param exclude_joint `integer` exclude packages which are dependencies of at least N other packages, not count main package dependencies. Default: `0`
+#' @param file `character` file name to read. Possible values are `DESCRIPTION`, `NEWS` and `NAMESPACE`.
 #' @param fields `character` vector listing the types of dependencies, a subset of `c("Depends", "Imports", "LinkingTo", "Suggests", "Enhances")`.
 #' Character string "all" is shorthand for that vector, character string "most" for the same vector without "Enhances", character string "strong" (default) for the first three elements of that vector.
 #' Default: `c("Depends", "Imports", "LinkingTo")`

diff --git a/man/cran_archive_file.Rd b/man/cran_archive_file.Rd
diff --git a/man/read_cran_file.Rd b/man/read_cran_file.Rd
diff --git a/man/read_github_file.Rd b/man/read_github_file.Rd
diff --git a/man/standard_args.Rd b/man/standard_args.Rd
diff --git a/tests/testthat/test-compare.R b/tests/testthat/test-compare.R
@@ -41,3 +41,18 @@ test_that("pacs::pac_comapre_namespace offline", {
   mockery::stub(pac_compare_namespace_offline, "is_online", FALSE)
   expect_error(pac_compare_namespace_offline("memoise", "0.2.1", "2.0.0"), "is_online\\(\\) is not TRUE")
 })
+
+
+test_that("pacs::pac_compare_news", {
+  skip_if_offline()
+  expect_identical(pac_compare_news("memoise", "2.0.0", "22.4.0"), NA)
+  expect_error(pac_compare_news("memoise", "22.8.0", "22.4.0"), "compareVersion")
+  expect_identical(pac_compare_news("WRONG"), NA)
+})
+
+test_that("pacs::pac_compare_news online", {
+  skip_if_offline()
+  expect_true(length(pac_compare_news("dplyr", "0.7.1", "1.0.0")) == 947)
+  expect_true(length(pac_compare_news("memoise", old = "1.0.0")) > 0)
+  expect_true(length(pac_compare_news("memoise", "1.0.0", "2.0.0")) > 0)
+})
diff --git a/tests/testthat/test-description.R b/tests/testthat/test-description.R
@@ -1,12 +1,3 @@
-test_that("cran_archive_file", {
-  skip_if_offline()
-  expect_true(length(cran_archive_file("dplyr", "1.0.0", "DESCRIPTION", "https://cran.rstudio.com/")) == 22)
-  expect_identical(
-    cran_archive_file("dplyr", "0.0.0.1", "DESCRIPTION", "https://cran.rstudio.com/"),
-    structure(list(), package = "dplyr", version = "0.0.0.1")
-  )
-})
-
 test_that("pacs::pac_description", {
   skip_if_offline()
   expect_true(length(pac_description("dplyr", version = "0.8.0")) == 23)

diff --git a/tests/testthat/test-namespace.R b/tests/testthat/test-namespace.R
@@ -8,8 +8,8 @@ test_that("pac_parse_namespace", {
 
 test_that("pacs::pac_namespace", {
   skip_if_offline()
-  expect_identical(pac_readnamespace_raw("dplyr", "0.0.0.0.1"), structure(list(), package = "dplyr", version = "0.0.0.0.1"))
-  expect_true(length(cran_archive_file("dplyr", "0.8.0", repos = "https://cran.rstudio.com/", "NAMESPACE")) == 498)
+  expect_identical(pac_readnamespace_raw("dplyr", "0.0.0.0.1"), structure(NA, package = "dplyr", version = "0.0.0.0.1"))
+  expect_true(length(read_cran_file("dplyr", "0.8.0", repos = "https://cran.rstudio.com/", "NAMESPACE")) == 498)
   expect_true(length(pac_namespace("dplyr", version = "0.8.0")) == 10)
   expect_identical(sort(pac_namespace("memoise", local = TRUE)$exports), sort(base::getNamespaceExports("memoise")))
   expect_identical(pac_namespace("dplyr", "1.1.1.1"), NA)

diff --git a/tests/testthat/test-read_cran.R b/tests/testthat/test-read_cran.R
@@ -0,0 +1,15 @@
+test_that("pacs:::read_cran_file valid input", {
+  skip_if_offline()
+  expect_silent(pacs:::read_cran_file("memoise", "1.1.0", "DESCRIPTION", repos = "https://cran.rstudio.com/"))
+  expect_silent(pacs:::read_cran_file("memoise", "1.1.0", "NAMESPACE", repos = "https://cran.rstudio.com/"))
+  expect_silent(pacs:::read_cran_file("memoise", "1.1.0", "NEWS"))
+})
+
+test_that("pacs::read_cran_file expected output", {
+  skip_if_offline()
+  expect_true(length(read_cran_file("dplyr", "1.0.0", "DESCRIPTION", "https://cran.rstudio.com/")) == 22)
+  expect_identical(
+    read_cran_file("dplyr", "0.0.0.1", "DESCRIPTION", "https://cran.rstudio.com/"),
+    NA
+  )
+})
diff --git a/tests/testthat/test-read_github.R b/tests/testthat/test-read_github.R
@@ -0,0 +1,13 @@
+
+test_that("pacs:::read_github_file valid input", {
+  skip_if_offline()
+  expect_silent(pacs:::read_github_file("memoise", "1.1.0", "DESCRIPTION", repos = "https://cran.rstudio.com/"))
+  expect_silent(pacs:::read_github_file("memoise", "1.1.0", "NAMESPACE", repos = "https://cran.rstudio.com/"))
+  expect_silent(pacs:::read_github_file("memoise", "1.1.0", "NEWS"))
+})
+
+test_that("pacs:::read_github_file invalid input", {
+  skip_if_offline()
+  expect_identical(pacs:::read_github_file("dplyr", "0.8.0.2", "DESCRIPTION", repos = "https://cran.rstudio.com/"),NA)
+  expect_error(pacs:::read_github_file("dplyr2", "0.8.0", "NAMESPACE", repos = "https://cran.rstudio.com/"), NA)
+})
diff --git a/tests/testthat/test-validate_input.R b/tests/testthat/test-validate_input.R
@@ -0,0 +1,20 @@
+test_that("test valid validate_pac_input input", {
+    expect_silent(validate_pac_input("memoise", version = NULL, at = NULL, local = TRUE, lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+    expect_silent(validate_pac_input("memoise", version = "1.1.0", at = NULL, local = FALSE, lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+})
+
+test_that("test invalid validate_pac_input input", {
+    expect_error(validate_pac_input("memoise", version = "1.1.0", at = NULL, local = TRUE, lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+    expect_error(validate_pac_input("memoise", version = "1.1.0", at = as.Date("2019-02-01"), local = FALSE, lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+})
+
+test_that("test valid validate_compare_input input", {
+    expect_silent(validate_compare_input("memoise", old = NULL, new = NULL, lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+    expect_silent(validate_compare_input("memoise", old = "1.1.0", new = "1.1.1", lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+})
+
+test_that("test invalid validate_compare_input input", {
+    expect_error(validate_compare_input(c("memoise", "sth"), old = "1.1.0", new = "1.1.1", lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+    expect_error(validate_compare_input("memoise", old = c("1.1.0", "1"), new = "1.1.1", lib.loc = .libPaths(), repos = "https://cran.rstudio.com/"))
+    expect_error(validate_compare_input("memoise", old = "1.1.0", new = "1.1.1", lib.loc = .libPaths(), repos = 2))
+})
diff --git a/tests/testthat/test-validate_pac_input.R b/tests/testthat/test-validate_pac_input.R