Merge pull request #191 from mrkaye97/adds-test-coverage

`slackr 3.3.1`

.github/workflows/test-coverage.yaml

@@ -2,7 +2,9 @@ on:
   push:
     branches:
       - master
-
+  pull_request:
+    branches:
+      - master
 name: test-coverage
 
 jobs:
@@ -48,5 +50,5 @@ jobs:
         shell: Rscript {0}
 
       - name: Test coverage
-        run: covr::codecov(quiet = FALSE)
+        run: covr::codecov(quiet = FALSE, function_exclusions = c("slackr_dev", "slackr_tex"))
         shell: Rscript {0}

DESCRIPTION

@@ -2,7 +2,7 @@ Type: Package
 Package: slackr
 Title: Send Messages, Images, R Objects and Files to 'Slack'
     Channels/Users
-Version: 3.3.0
+Version: 3.3.1
 Author: Bob Rudis [aut, cre], Jay Jacobs [ctb], David Severski [ctb],
     Quinn Weber [ctb], Konrad Karczewski [ctb], Shinya Uryu [ctb], Gregory
     Jefferis [ctb], Ed Niles [ctb], Rick Saporta [ctb], Jonathan Sidi
@@ -27,7 +27,6 @@ Depends:
 Imports:
     cachem (>= 1.0.4),
     dplyr,
-    graphics,
     grDevices,
     httr (>= 1.4.2),
     jsonlite,

NEWS.md

@@ -1,3 +1,8 @@
+# slackr 3.3.1
+* Fixes a bug in `slackr_history` where the function fails to infer `posted_from_time` if not provided
+* Adds default `message_count = 100` to `slackr_history`
+* Improves test coverage
+
 # slackr 3.3.0
   User-facing changes:
 

R/slackr_history.R

@@ -24,15 +24,15 @@
 #' @param message_count The number of messages to retrieve (only when `paginate = FALSE`).
 #'   Corresponds to `limit` in the \href{https://api.slack.com/methods/conversations.history}{conversations.history} docs.
 #'   Note: If using pagination, setting a value of (e.g.) 1 will result in paginating
-#'   through the channel's history one message at a time. Slack recommends using a value of 200.
+#'   through the channel's history one message at a time. Slack recommends using a value of 200. (Default: 100)
 #' @param inclusive Include messages with oldest or latest timestamps in results. Ignored unless either timestamp is specified.
 #' @export
 #'
 #' @return A `tibble` with message metadata
 #' @references <https://api.slack.com/methods/conversations.history>
 #'
 slackr_history <- function(
-  message_count,
+  message_count = 100,
   channel = Sys.getenv("SLACK_CHANNEL"),
   token = Sys.getenv("SLACK_TOKEN"),
   posted_to_time = as.numeric(Sys.time()),
@@ -60,7 +60,7 @@ slackr_history <- function(
         .frequency_id = "slackr_history_posted_from_infer_warning"
       )
     }
-    posted_from_time <- posted_to_time - duration * 3600
+    posted_from_time <- as.character(as.numeric(posted_to_time) - duration * 3600)
   }
 
   if (!paginate) {

codecov.yml

@@ -1,5 +1,4 @@
 comment: false
-
 coverage:
   status:
     project:

tests/testthat/test-dev-tex.R

@@ -10,12 +10,26 @@ test_that("slackr_dev posts", {
   expect_equal(post$file$filetype, "png")
 })
 
-test_that("slackr_tex posts", {
+test_that("slackr_tex posts with no dir specified", {
   skip_on_cran()
   skip_on_ci()
 
+  ## No path specified
   post <- slackr_tex("$$e^{i \\pi} + 1 = 0$$")
 
   expect_true(post$ok)
   expect_equal(post$file$filetype, "png")
 })
+
+test_that("slackr_tex posts with dir specified", {
+  skip_on_cran()
+  skip_on_ci()
+
+  ## With path specified
+  d <- tempdir()
+  post <- slackr_tex("$$e^{i \\pi} + 1 = 0$$", path = d)
+
+  unlink(d, recursive = TRUE)
+  expect_true(post$ok)
+  expect_equal(post$file$filetype, "png")
+})

tests/testthat/test-history.R

@@ -120,7 +120,7 @@ test_that("slackr_history works when posted_from and posted_to are specified for
   )
 })
 
-test_that("Specifycing post times in slackr_history correctly limits time window", {
+test_that("Specifying post times in slackr_history correctly limits time window", {
   skip_on_cran()
 
   lapply(
@@ -148,3 +148,40 @@ test_that("Specifycing post times in slackr_history correctly limits time window
     }
   )
 })
+
+test_that("Expect warning for too many params", {
+  skip_on_cran()
+
+  from_time <- slackr_msg("History warning test start")$ts
+  Sys.sleep(1)
+  slackr_msg("History warning test")
+  Sys.sleep(1)
+  to_time <- slackr_msg("History warning test end")$ts
+
+  expect_silent(
+    slackr_history(
+      posted_to_time = to_time,
+      duration = 2 * (as.numeric(to_time) - as.numeric(from_time)) / (60 * 60)
+    )
+  )
+
+  expect_warning(
+    slackr_history(
+      posted_from_time = from_time,
+      posted_to_time = to_time,
+      duration = 2 * (as.numeric(to_time) - as.numeric(from_time)) / (60 * 60)
+    ),
+    "You specified all three of"
+  )
+})
+
+test_that("Error if trying to paginate with no start time", {
+  skip_on_cran()
+
+  expect_error(
+    slackr_history(
+      paginate = TRUE
+    ),
+    "To use pagination with `slackr_history`, you must specify a value for `posted_from_time`"
+  )
+})

tests/testthat/test-internals.R

@@ -74,3 +74,32 @@ test_that("Users works", {
     "mrkaye97" %in% users$name
   )
 })
+
+test_that("with_retry correctly retries requests", {
+  skip_on_cran()
+
+  i <- 1
+
+  mock <- function() {
+    if (i > 1) {
+      httr:::response(
+        headers = list(`retry-after` = 2),
+        status_code = 200,
+        ok = TRUE
+      )
+    } else {
+      i <<- i + 1
+      httr:::response(
+        headers = list(`retry-after` = 2),
+        status_code = 429
+      )
+    }
+  }
+
+  expect_message(with_retry(mock), "Pausing for 2 seconds due to Slack API rate limit")
+
+  out <- with_retry(mock)
+
+  expect_true(out$ok)
+  expect_equal(out$status_code, 200)
+})

tests/testthat/test-onexit.R

@@ -0,0 +1,13 @@
+test_that("onexit usage works as expected", {
+  skip_on_cran()
+
+  slack_lm <- register_onexit(lm, "testing onexit")
+
+  formula <- Sepal.Length ~ Sepal.Width
+  mod <- slack_lm(formula, data = iris)
+
+  expect_identical(
+    mod$coefficients,
+    lm(formula, data = iris)$coefficients
+  )
+})