-
-
Notifications
You must be signed in to change notification settings - Fork 219
skip tests with path issues on Windows #1258
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
Conversation
You could probably resolve the Windows issues by using I'm also fine with just not worrying about it though, as long as we're certain it's an issue with the test itself and not with Rcpp (which appears to be the case). |
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.
Changeset is great
Happy to merge as is, question if we should try the suggestion by @kevinushey ? Or not bother? |
I think we shouldn't bother. Those tests already were "let's be super cautious" tests. They pass on Unixes, then everything is fine. |
We'll know how r-universe fares in a bit. I'll report back. |
Our CI failed on the very regular / unchanged 'update container' step. Has to do with Debian experimental changing its name to rc-buggy so for now I am just going to remove the apt list file. |
And in the meantime r-universe caught up and we are now four out of five: The Apple 4.3 fail is my overeager switch to a higher precision qnorm comparison:
and I'll reverse that out and make sure we only compare to a lower tolerance of, say, 1e-6 > all.equal(-447.197893678525, -447.19749446504, tol=1e-5)
[1] TRUE
> all.equal(-447.197893678525, -447.19749446504, tol=1e-6)
[1] TRUE
> all.equal(-447.197893678525, -447.19749446504, tol=1e-7)
[1] "Mean relative difference: 8.927e-07"
> |
The fix in 54ee8fd uncovered an issue with those two tests on Windows. Life is too short to debug path issues for tests on Windows, so this fix just skips them there.
Successful rhub tests here (there's one ERROR, but it's about vignette rebuilding, something about
texi2pdf
not being found): https://builder.r-hub.io/status/Rcpp_1.0.10.3.tar.gz-94d653c1f4064cf29639fc441187ce3dChecklist
R CMD check
still passes all tests