You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@arrow.apache.org by "Dragoș Moldovan-Grünfeld (Jira)" <ji...@apache.org> on 2022/01/17 10:17:00 UTC

[jira] [Comment Edited] (ARROW-15301) [R] Discussion: move testthat test helpers to R/test-helpers.R

    [ https://issues.apache.org/jira/browse/ARROW-15301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17474541#comment-17474541 ] 

Dragoș Moldovan-Grünfeld edited comment on ARROW-15301 at 1/17/22, 10:16 AM:
-----------------------------------------------------------------------------

I wouldn't regard moving all {{testthat}} helpers to {{test-helpers.R}} or some file that ends with _helpers_ as a full-on reorganisation.

Why a file ending in {_}helpers{_}? Because {{devtools::load_all()}} sources (via {{pkgload:::testthat_source_test_helpers()}} all files ending in _helpers_ and, thus, any functions defined there are available interactively. In the current scenario if a developer needs to run a test block containing an \{arrow}-specific helper function, they need to find where the helper is defined (source that file or run the lines) and then run the test block. I think this workflow adds quite a bit of friction.



was (Author: dragosmg):
I wouldn't regard moving all {{testthat}} helpers to {{test-helpers.R}} or some file that ends with _helpers_ as a full-on reorganisation.

-Why a file ending in {_}helpers{_}? Because {{devtools::load_all()}} sources (via {{pkgload:::testthat_source_test_helpers()}} all files ending in _helpers_ and, thus, any functions defined there are available interactively. In the current scenario if a developer needs to run a test block containing an \{arrow}-specific helper function, they need to find where the helper is defined (source that file or run the lines) and then run the test block. I think this workflow adds quite a bit of friction.-

My understanding of loading helpers was incorrect. I think {{pkgload::testthat_source_test_helpers()}} sources all files that start with the word {{helper}} from the test directory ({{tests/testthat}}).

> [R] Discussion: move testthat test helpers to R/test-helpers.R
> --------------------------------------------------------------
>
>                 Key: ARROW-15301
>                 URL: https://issues.apache.org/jira/browse/ARROW-15301
>             Project: Apache Arrow
>          Issue Type: Improvement
>          Components: R
>            Reporter: Dragoș Moldovan-Grünfeld
>            Priority: Major
>             Fix For: 8.0.0
>
>
> Would it make sense to have all custom expectations in {{R/}} , maybe {{R/test-helpers.R}} ? Reasons: 
>  
>  * edit(added on 12/01/2022): probably the most important point is these files are source by {{devtools::load_all()}} and, thus, helper functions are available interactively. 
>  * easier for newbies to read a file and figure out what custom testthat helpers we have
>  * easier to document
>  * respects the testthat author's suggestion to have them there
> Currently we have 17 different custom expectations, all defined in different files.
> A follow-up thought: maybe move all helpers there ({{{}is.{}}}, {{{}as_{}}}, {{{}compare_{}}}, {{verify}} etc).



--
This message was sent by Atlassian Jira
(v8.20.1#820001)