Skip to content
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

GH-37804: [R] Fix with_language test helper #37811

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nealrichardson
Copy link
Member

@nealrichardson nealrichardson commented Sep 20, 2023

Rationale for this change

Proper fix for #37804. Turns out the with_language() helper was wrong, but usually in an innocuous way that would make it skip more than it should.

Are these changes tested?

Yes, though we'll want to watch the nightlies and may need to add another nightly that uses rocker/r-devel in order to ensure (such that we can) that whatever changed on CRAN is covered. It's not clear that the R-hub job we test that uses R devel is getting updated nightly anymore.

Are there any user-facing changes?

No.

@thisisnic
Copy link
Member

Now this is being really weird and passing on Windows (and passes on MacOS) but failing on Ubuntu 🙃

@thisisnic thisisnic changed the title MINOR: [R] Fix with_language test helper GH-37810: [R] Fix with_language test helper Oct 4, 2023
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

⚠️ GitHub issue #37810 has been automatically assigned in GitHub to PR creator.

@thisisnic thisisnic changed the title GH-37810: [R] Fix with_language test helper GH-37804: [R] Fix with_language test helper Oct 4, 2023
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

⚠️ GitHub issue #37804 has been automatically assigned in GitHub to PR creator.

@thisisnic
Copy link
Member

I'm wondering if this could be relevant here? r-lib/withr#213

@nealrichardson
Copy link
Member Author

Maybe? Note that the with_language() here is not the same as withr::with_language(), and we explicitly look for the case where setting LANG does not result in changing the translation and skip the test if not. It's possible that extra check is wrong--it seemed to be wrong before this PR, but may be wrong differently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[R] Tests which use with_language() fail on CRAN
3 participants