thisisnic commented on a change in pull request #10624:
URL: https://github.com/apache/arrow/pull/10624#discussion_r663049290



##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -866,3 +866,57 @@ test_that("str_like", {
     df
   )
 })
+
+test_that("substrings", {
+  df <- tibble(
+    x = "Apache Arrow"
+  )
+
+  expect_dplyr_equal(
+    input %>%
+      mutate(
+        foo = "Apache Arrow",
+        bar1 = substr(foo, 1, 6), # Apache

Review comment:
       Please can you split these out into separate tests?  It makes it easier 
to follow and fix issues.

##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -866,3 +866,57 @@ test_that("str_like", {
     df
   )
 })
+
+test_that("substrings", {
+  df <- tibble(
+    x = "Apache Arrow"
+  )
+
+  expect_dplyr_equal(
+    input %>%
+      mutate(
+        foo = "Apache Arrow",
+        bar1 = substr(foo, 1, 6), # Apache
+        bar2 = substr(foo, 0, 6), # Apache
+        bar3 = substr(foo, -1, 6), # Apache
+        bar4 = substr(foo, 6, 1), # ""
+        bar5 = substr(foo, -1, -2), # ""
+        bar6 = substr(foo, 8, 12) # Arrow
+      ) %>%
+      select(bar1:bar6) %>%

Review comment:
       ```suggestion
   
   ```

##########
File path: r/tests/testthat/test-dplyr-string-functions.R
##########
@@ -866,3 +866,57 @@ test_that("str_like", {
     df
   )
 })
+
+test_that("substrings", {
+  df <- tibble(
+    x = "Apache Arrow"
+  )
+
+  expect_dplyr_equal(
+    input %>%
+      mutate(
+        foo = "Apache Arrow",
+        bar1 = substr(foo, 1, 6), # Apache
+        bar2 = substr(foo, 0, 6), # Apache
+        bar3 = substr(foo, -1, 6), # Apache
+        bar4 = substr(foo, 6, 1), # ""
+        bar5 = substr(foo, -1, -2), # ""
+        bar6 = substr(foo, 8, 12) # Arrow

Review comment:
       No need to set up string "Apache Arrow" via variable `foo` as it's 
already been set up as variable `x`.
   ```suggestion
           bar1 = substr(x, 1, 6), # Apache
           bar2 = substr(x, 0, 6), # Apache
           bar3 = substr(x, -1, 6), # Apache
           bar4 = substr(x, 6, 1), # ""
           bar5 = substr(x, -1, -2), # ""
           bar6 = substr(x, 8, 12) # Arrow
   ```




-- 
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