Skip to content

Use waldo::compare(list_as_map) in expect_mapequal() #2150

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 1 commit into
base: main
Choose a base branch
from

Conversation

hadley
Copy link
Member

@hadley hadley commented Jul 31, 2025

This weakens the expectation so it shouldn't cause any existing tests to fail.

Fixes #1521

This weakens the expectation so it shouldn't cause any existing tests to fail.

Fixes #1521
expect_warning(expect_success(expect_mapequal(x2, x1)))
expect_warning(expect_success(expect_mapequal(x2, x2)))
test_that("fails if unnamed values in different location if any unnamed values", {
expect_success(expect_mapequal(list(1, b = 2, c = 3), list(1, c = 3, b = 2)))

Choose a reason for hiding this comment

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

Should unnamed values be allowed at all in expect_mapequal()?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think waldo switches to "not a map" if there are any unnamed values, which seems reasonable to me. It will cause a failure, rather than an error, but I don't think that makes much practical difference.

Choose a reason for hiding this comment

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

So that should mean that this test should fail, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmmm, ok that's not actually what waldo does — when it turns a list into a map, it removes all NULLs, and then reordered named elements, preserving the location of unnamed. That's a different choice but I think equally valid.

Choose a reason for hiding this comment

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

Perhaps so. Maybe this is moot because the use case I think of for mapequal is comparing something coming in from a JSON object, and {1, "b": 2} is not valid JSON. So I would never have a test like this line, and thus it doesn't matter what expect_mapequal() does in this case.

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.

Should expect_mapequal() be recursive?
2 participants