From 632822249506c4738f99761d8b3ae30bdd22dac1 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 5 Nov 2024 16:41:15 -0600 Subject: [PATCH 1/2] Correct direction of `expect_setequal()` failure message Fixes #1962 --- NEWS.md | 1 + R/expect-setequal.R | 4 ++-- tests/testthat/_snaps/expect-setequal.md | 17 ++++++++++++----- tests/testthat/test-expect-setequal.R | 3 ++- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1f595bb41..fa4c464cc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* `expect_setequal()` correctly identifies what is missing where (#1962). * `expect_true()` and `expect_false()` give better errors if `actual` isn't a vector (#1996). * `expect_no_*()` expectations no longer incorrectly emit a passing test result if they in fact fail (#1997). * Require the latest version of waldo (0.6.0) in order to get the latest goodies (#1955). diff --git a/R/expect-setequal.R b/R/expect-setequal.R index d710a1ac7..42b6aa7c0 100644 --- a/R/expect-setequal.R +++ b/R/expect-setequal.R @@ -42,9 +42,9 @@ expect_setequal <- function(object, expected) { fail(paste0( act$lab, " (`actual`) and ", exp$lab, " (`expected`) don't have the same values.\n", if (any(act_miss)) - paste0("* Only in `expected`: ", values(act$val[act_miss]), "\n"), + paste0("* Only in `actual`: ", values(act$val[act_miss]), "\n"), if (any(exp_miss)) - paste0("* Only in `actual`: ", values(exp$val[exp_miss]), "\n") + paste0("* Only in `expected`: ", values(exp$val[exp_miss]), "\n") )) } else { succeed() diff --git a/tests/testthat/_snaps/expect-setequal.md b/tests/testthat/_snaps/expect-setequal.md index 1225d9914..3ea8247ad 100644 --- a/tests/testthat/_snaps/expect-setequal.md +++ b/tests/testthat/_snaps/expect-setequal.md @@ -1,26 +1,33 @@ # useful message on failure + "actual" (`actual`) and "expected" (`expected`) don't have the same values. + * Only in `actual`: "actual" + * Only in `expected`: "expected" + + +--- + 1:2 (`actual`) and 2 (`expected`) don't have the same values. - * Only in `expected`: 1 + * Only in `actual`: 1 --- 2 (`actual`) and 2:3 (`expected`) don't have the same values. - * Only in `actual`: 3 + * Only in `expected`: 3 --- 1:2 (`actual`) and 2:3 (`expected`) don't have the same values. - * Only in `expected`: 1 - * Only in `actual`: 3 + * Only in `actual`: 1 + * Only in `expected`: 3 # truncates long vectors 1:2 (`actual`) and 1:50 (`expected`) don't have the same values. - * Only in `actual`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ... + * Only in `expected`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ... # expect_contains() gives useful message on failure diff --git a/tests/testthat/test-expect-setequal.R b/tests/testthat/test-expect-setequal.R index b4de30800..6966822ec 100644 --- a/tests/testthat/test-expect-setequal.R +++ b/tests/testthat/test-expect-setequal.R @@ -24,6 +24,8 @@ test_that("error for non-vectors", { }) test_that("useful message on failure", { + expect_snapshot_failure(expect_setequal("actual", "expected")) + expect_snapshot_failure(expect_setequal(1:2, 2)) expect_snapshot_failure(expect_setequal(2, 2:3)) expect_snapshot_failure(expect_setequal(1:2, 2:3)) @@ -112,4 +114,3 @@ test_that("expect_in() gives useful message on failure", { expect_snapshot_failure(expect_in(x1, x2)) expect_snapshot_failure(expect_in(x1, x3)) }) - From c819edc2a023db6214e99018534461e45f771638 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 5 Nov 2024 16:43:29 -0600 Subject: [PATCH 2/2] Make code easier to understnad --- R/expect-setequal.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/R/expect-setequal.R b/R/expect-setequal.R index 42b6aa7c0..cfca84654 100644 --- a/R/expect-setequal.R +++ b/R/expect-setequal.R @@ -35,16 +35,16 @@ expect_setequal <- function(object, expected) { warn("expect_setequal() ignores names") } - act_miss <- !act$val %in% exp$val - exp_miss <- !exp$val %in% act$val + act_miss <- setdiff(act$val, exp$val) + exp_miss <- setdiff(exp$val, act$val) - if (any(exp_miss) || any(act_miss)) { + if (length(exp_miss) || length(act_miss)) { fail(paste0( act$lab, " (`actual`) and ", exp$lab, " (`expected`) don't have the same values.\n", - if (any(act_miss)) - paste0("* Only in `actual`: ", values(act$val[act_miss]), "\n"), - if (any(exp_miss)) - paste0("* Only in `expected`: ", values(exp$val[exp_miss]), "\n") + if (length(act_miss)) + paste0("* Only in `actual`: ", values(act_miss), "\n"), + if (length(exp_miss)) + paste0("* Only in `expected`: ", values(exp_miss), "\n") )) } else { succeed()