r-lib / styler

Non-invasive pretty printing of R code
https://styler.r-lib.org
Other
713 stars 70 forks source link

Style `test_that()` calls to make them more BDD/readability-friendly? #1168

Open IndrajeetPatil opened 9 months ago

IndrajeetPatil commented 9 months ago

Preamble

The {testthat} tests read like sentences (à la BDD), and when the description is in the same line as the function name, this can really boost its readability. For example, the following:

test_that("multiplication works", {
   expect_equal(2 * 2, 4)
})

can be read mentally as

"test that multiplication works as expected"

and the formatting helps this mental dialogue.

Actual vs Expected

But when the code is styled differently (e.g. using Allman-style indentation),

library(styler)

'test_that(
  "multiplication works",
  {
    expect_equal(2 * 2, 4)
  }
)' -> code

style_text(code)
#> test_that(
#>   "multiplication works",
#>   {
#>     expect_equal(2 * 2, 4)
#>   }
#> )

{styler} doesn't restyle it to its more readable version:

#> test_that("multiplication works", {
#>   expect_equal(2 * 2, 4)
#> })

Do you think {styler} should by default restyle such calls? Or doing so violates its "non-invasive" nature?

FWIW, the style guide doesn't seem to have anything to say in this matter (except indirectly).

Exception

This is feature request is relevant only when there are no keyword arguments, only positional arguments, are present in the test_that() call.

'test_that(
  desc = "multiplication works",
  code = {
    expect_equal(2 * 2, 4)
  }
)' -> code

style_text(code)
#> test_that(
#>   desc = "multiplication works",
#>   code = {
#>     expect_equal(2 * 2, 4)
#>   }
#> )
lorenzwalthert commented 9 months ago

I see the point and I was reading up on https://github.com/r-lib/styler/issues/549 where we had a similar discussion. I believe that if you can turn an expression into the compact form, e.g. if the result is

test_that('x', {
  # stuff
})

we should do that. Compact form means that all opening brackets are on the same line and all closing brackets are on the same line after the transformation.

For example your code:

test_that(
  "multiplication works",
  {
    expect_equal(2 * 2, 4)
  }
)

# can turn into  
test_that("multiplication works", { # both opening braces on same line
  expect_equal(2 * 2, 4) 
}) # both closing braces on same line

Similarly here

tryCatch(
  {
    expect_equal(2 * 2, 4)
  }
) 
# can turn into 
tryCatch({
  expect_equal(2 * 2, 4)
})

Hence, I don't think that's {testthat} specific.