r-lib / testthat

An R 📦 to make testing 😀
https://testthat.r-lib.org
Other
891 stars 316 forks source link

Should expect_mapequal() be recursive? #1521

Open nealrichardson opened 2 years ago

nealrichardson commented 2 years ago

It seems only to make unordered comparisons at the top level. I would expect nested lists to get the same mapequal treatment.

testthat::expect_mapequal(
  list(
    outer_1 = 1,
    outer_2 = list(
      inner_1 = 1,
      inner_2 = 2
    )
  ),
  list(
    outer_2 = list(
      inner_2 = 2,
      inner_1 = 1
    ),
    outer_1 = 1
  )
)
#> Error: act$val[exp_nms] not equal to exp$val.
#> Component "outer_2": Names: 2 string mismatches
#> Component "outer_2": Component 1: Mean relative difference: 1
#> Component "outer_2": Component 2: Mean relative difference: 0.5

Created on 2021-12-31 by the reprex package (v2.0.1)

hadley commented 2 years ago

Hmmm, I'll need to think about whether it's worth changing expect_mapequal() or just advertising list_as_map = TRUE more.

library(testthat)

test_that("can recursively ignore differences", {
  local_edition(3)
  expect_equal(
    list(
      outer_1 = 1,
      outer_2 = list(
        inner_1 = 1,
        inner_2 = 2
      )
    ),
    list(
      outer_2 = list(
        inner_2 = 2,
        inner_1 = 1
      ),
      outer_1 = 1
    ),
    list_as_map = TRUE
  )

})
#> Test passed 🌈

Created on 2022-01-02 by the reprex package (v2.0.1)

nealrichardson commented 2 years ago

Oh cool, I didn't know about the list_as_map argument, that's nice. Though I'm surprised that expect_mapequal() behaves differently and isn't just expect_equal(list_as_map = TRUE).

hadley commented 2 years ago

Yeah. The main thought that’s required if it can just change that behaviour for 3e. It probably isn’t used very often so it’s likely to be possible but it’ll need a little research and to wait for a minor release.

hadley commented 2 years ago

Usage looks pretty minimal, so the change is likely to be fine: https://cs.github.com/?scopeName=All+repos&scope=&q=language%3Ar%20expect_mapequal%20-repo%3Ar-lib%2Ftestthat