Skip to content

Don't assume from="UTF-8" in encoding test - #7852

Merged
aitap merged 4 commits into
masterfrom
utf8-old-windows
Jul 30, 2026
Merged

Don't assume from="UTF-8" in encoding test#7852
aitap merged 4 commits into
masterfrom
utf8-old-windows

Conversation

@MichaelChirico

@MichaelChirico MichaelChirico commented Jul 28, 2026

Copy link
Copy Markdown
Member

For #7217 (possibly closing it?). As described by Ivan below.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.01%. Comparing base (dcaa87c) to head (8fe3782).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7852   +/-   ##
=======================================
  Coverage   99.01%   99.01%           
=======================================
  Files          88       88           
  Lines       17286    17286           
=======================================
  Hits        17116    17116           
  Misses        170      170           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aitap

aitap commented Jul 28, 2026

Copy link
Copy Markdown
Member

Here, the test is being overly pessimistic. If the native encoding is capable of representing all the needed characters, it should be possible for the test to pass. y just needs the correctly converted names, not from UTF-8, but from the native encoding: 52271bb

This looks extremely counter-intuitive. setnames(y , iconv(c("\u00f6", "\u00fc", "\u00e4"), from = "UTF-8", to = "latin1")) on its own would be right (string literals with Unicode escapes do get marked as UTF-8 as intended), but here we're doing eval(parse(text = 'iconv(...)')). In order to parse the source code, R first converts the Unicode source code into the native encoding. After parse(), we're evaluating iconv(c("ö", "ü", "ä"), to = 'latin1'), which must use from = '' (default), not from = "UTF-8".

@MichaelChirico

Copy link
Copy Markdown
Member Author

makes sense... maybe utf8_check() isn't the perfect name for the helper? I'm still wondering why this is only an issue for ancient windows

@aitap

aitap commented Jul 28, 2026

Copy link
Copy Markdown
Member

True, the check is more like representable_in_locale_encoding().

Almost everywhere else except on old R or old Windows the locale encoding is UTF-8, so iconv(from = "UTF-8") instead of from = "" does not break anything. You can also break this test on Linux using a non-Unicode locale that represents the necessary umlauts (say, de_DE.ISO-8859-1).

@MichaelChirico MichaelChirico changed the title Extend the UTF-8 check for tests 2298* on old Windows Don't assume from="UTF-8" in encoding test Jul 28, 2026
@MichaelChirico
MichaelChirico force-pushed the as-date-names-old-r branch 2 times, most recently from 2eedafe to e737a46 Compare July 28, 2026 20:25
@MichaelChirico
MichaelChirico force-pushed the utf8-old-windows branch 2 times, most recently from 40bd8d6 to a0a8500 Compare July 28, 2026 20:26
@MichaelChirico
MichaelChirico changed the base branch from as-date-names-old-r to master July 28, 2026 20:48
@MichaelChirico MichaelChirico added encoding issues related to Encoding tests labels Jul 28, 2026
@aitap
aitap merged commit 7373a7a into master Jul 30, 2026
20 of 24 checks passed
@aitap
aitap deleted the utf8-old-windows branch July 30, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

encoding issues related to Encoding tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants