Skip to content

Add more informative feedback in expect_named() #2130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

EmilHvitfeldt
Copy link
Member

@EmilHvitfeldt EmilHvitfeldt commented Jul 28, 2025

Hope to close #2091 and close #1558

This PR aims to add more informative errors.

library(testthat)

x <- setNames(nm = 1:100)

expect_named(x, as.character(c(1:56, 58:100)), ignore.order = TRUE)
#> Error: Names of `x` (`actual`) and as.character(c(1:56, 58:100)) (`expected`) don't have the same values.
#> * Only in `actual`: "57"

Created on 2025-07-28 with reprex v2.1.1

R/expect-named.R Outdated
act_miss <- unique(act$names[!act$names %in% exp$val])
exp_miss <- unique(exp$val[!exp$val %in% act$names])

expect(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expect() block is copied and modified slightly from expect_setequal() to include "Names of ".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explore extracting out a single helper function that we could use here, in expect_setequal() and expect_mapequal()? A expectation helper should end in _ and have an argument trace_env = caller_env() that is passed to every use of fail().

@EmilHvitfeldt
Copy link
Member Author

@hadley for the ignore.cases = FALSE (default case) I was thinking that the waldo::compare() that powers expect_identical() would provide a high value reporting of the issues here. If you agree would I need to include if (edition_get() >= 3) {?

@hadley
Copy link
Member

hadley commented Jul 29, 2025

Can you hold off a day or two until I've finished refactoring how we write expectations? I'll have a better set of recommendations then.

@hadley
Copy link
Member

hadley commented Jul 30, 2025

Ok, can you please take another stab at it, following the updated custom-expectations vignette? That will also help me test if I've documented it adequately 😄

@@ -126,7 +126,8 @@ expect_waldo_equal_ <- function(
exp,
info = NULL,
...,
trace_env = caller_env()
trace_env = caller_env(),
error_prefix = NULL
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of error_prefix here and in expect_setequal_() are done to avoid duplication of these function by allowing me to add "Names of ", otherwise I would need some gnarly $lab modifications

@EmilHvitfeldt EmilHvitfeldt marked this pull request as ready for review August 1, 2025 23:53
@EmilHvitfeldt EmilHvitfeldt requested a review from hadley August 2, 2025 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give more informative message with expect_named() Using expect_named with glue causes error
2 participants