-
Notifications
You must be signed in to change notification settings - Fork 186
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
Wherever possible, prefer expect_identical()
over expect_equal()
#1712
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1712 +/- ##
=======================================
Coverage 98.77% 98.77%
=======================================
Files 104 104
Lines 4497 4497
=======================================
Hits 4442 4442
Misses 55 55
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
tests/testthat/test-methods.R
Outdated
df, | ||
exp | ||
) | ||
expect_equal(df, exp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's going on here? 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
df
columns are not of integer type here:
── Failure (test-methods.R:74): as.data.frame.lints ────────────────────────────
`df` (`actual`) not identical to `exp` (`expected`).
`actual$line_number` is a double vector (1, 2)
`expected$line_number` is an integer vector (1, 2)
`actual$column_number` is a double vector (1, 6)
`expected$column_number` is an integer vector (1, 6)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was going to fix it in a different PR, but might as well do it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, I would have suggested just leaving the style fix for that other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So should I undo this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no it's fine, minor point here
madman, you actually did it! 🤟🤟 |
expect_identical_linter()
lintsexpect_identical()
over expect_equal()
No description provided.