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

Ignore srcrefs by default in expect_identical() #714

Closed
lionel- opened this Issue Feb 9, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@lionel-
Member

lionel- commented Feb 9, 2018

cc @kalibera @hadley

And forward ... to identical() for more control.

@jimhester

This comment has been minimized.

Member

jimhester commented Mar 29, 2018

R 3.4.0 and up has a default of identical(ignore.srcref = TRUE). Prior to that there is no option. It seems unwise for testthat to mess with the objects source references in expect_identical() so I don't think there is much else to do other than pass ... to identical().

@jimhester jimhester added the feature label Mar 29, 2018

@jimhester jimhester closed this in 9fe404b Mar 29, 2018

@lionel-

This comment has been minimized.

Member

lionel- commented Mar 29, 2018

Compatibility aside I think it'd be preferable to ignore the srcrefs attributes by default since their presence depends on things like the editor. I wouldn't call it unwise. Tomas plans to enable srcrefs by default and that might cause check failures:

I found that rlang fails its own tests when packages have source references (now can be enabled with R_KEEP_PKG_SOURCE=yes and re-installing packages/rebuilding R, but I would like to make this a default, if the space and performance overhead is small enough)

I see very few use cases for testing presence and equality of srcrefs attributes.

@jimhester

This comment has been minimized.

Member

jimhester commented Mar 29, 2018

They are ignored by default in 3.4 and up, so if they are turned on in the future they will already be ignored.

@lionel-

This comment has been minimized.

Member

lionel- commented Mar 29, 2018

Ah that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment