jonkeane commented on code in PR #12867:
URL: https://github.com/apache/arrow/pull/12867#discussion_r849624997


##########
r/tests/testthat/test-dplyr-summarize.R:
##########
@@ -341,10 +337,9 @@ test_that("median()", {
     tbl,
     warning = "median\\(\\) currently returns an approximate median in Arrow"
   )
-  local_edition(3)
-})
+}, classes = "arrow.median.approximate"))

Review Comment:
   OOOH, that's fantastic. I don't think I knew that `suppressWarnings()` would 
let you specify classes, that's really nice!



##########
r/tests/testthat/test-dplyr-summarize.R:
##########
@@ -290,16 +290,12 @@ test_that("Functions that take ... but we only accept a 
single arg", {
   expect_error(call_binding_agg("max", 1, 2), "Multiple arguments to max()")
 })
 
-test_that("median()", {
+test_that("median()", suppressWarnings({

Review Comment:
   I like that we're not repeating `suppressWarnings()` a bunch, but I wonder 
if it might be better in this case to wrap each `compare_dplyr_binding()` 
separately so it's a bit close to the code we know will be generating too many 
warnings for us. I'm on the fence and fine to do either way — totally up to you!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to