Skip to content

Conversation

@hadley
Copy link
Member

@hadley hadley commented Oct 6, 2025

Fixes #1836. Fixes #2235. Closes #2240.

@claude
Copy link

claude bot commented Oct 6, 2025

Claude finished @hadley's task —— View job


Code Review

  • Review new expect_all_equal() implementation
  • Check test coverage and snapshots
  • Review modifications to expect-match.R
  • Identify potential bugs or issues
  • Provide minimal feedback

Analysis

The PR introduces expect_all_equal() - a function to test that all elements in a vector equal a single expected value. Implementation looks solid:

  • Input validation is comprehensive (R/expect-all.R:5-13)
  • Uses existing expect_waldo_equal_() infrastructure correctly
  • Test coverage is good with edge cases and error scenarios
  • Snapshots show appropriate error messages

The refactoring of show_text() in expect-match.R improves the API by making the labels parameter more generic.

LGTM.

@@ -0,0 +1,67 @@
#' Do you expect every value in a vector to have this value?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#' Do you expect every value in a vector to have this value?
#' Does a vector have a constant value?

Just thinking out loud

Copy link
Member Author

Choose a reason for hiding this comment

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

Constant makes me think unchanging (over time)

expect_all_equal(x, TRUE)
Condition
Error:
! Expected every element of `x` to equal TRUE.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
! Expected every element of `x` to equal TRUE.
! Expected every element of `x` to equal `TRUE`.

??

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW that would be inconsistent with expect_equal().

@hadley hadley changed the title WIP on expect_all_equal() Implement expect_all_equal() and friends Oct 8, 2025
@hadley hadley merged commit fc95be8 into main Oct 8, 2025
13 checks passed
@hadley hadley deleted the expect-all-equal branch October 8, 2025 19:22
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.

Suggest a new expectation around "Vector with a constant value" all= argument for expect_true()/expect_false()

2 participants