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

Snapshot error on Windows (encoding problem) #1574

Closed
maelle opened this issue Feb 15, 2022 · 4 comments
Closed

Snapshot error on Windows (encoding problem) #1574

maelle opened this issue Feb 15, 2022 · 4 comments
Labels
bug an unexpected problem or unintended behavior snapshot 📷
Milestone

Comments

@maelle
Copy link
Contributor

maelle commented Feb 15, 2022

In this reprex package https://github.com/maelle/encoding.problem I have a test

test_that("multiplication works", {
  bla <- list(someone = "Maëlle")
  expect_snapshot_output(print(bla))
})

I created the snapshot on Ubuntu. On Windows the test fails https://github.com/maelle/encoding.problem/actions/runs/1845617963

With error

> test_check("encoding.problem")
Error in gsub("\n", paste0("\n", prefix), x, fixed = TRUE) : 
Error: Error: R CMD check found ERRORs
  input string 1 is invalid UTF-8
Calls: test_check ... vapply -> FUN -> paste0 -> indent_add -> paste0 -> gsub
Execution halted

Which comes from indent_add() and functions it calls.

@cderv
Copy link

cderv commented Feb 15, 2022

I have also encountered this error after the last testthat release.

At the time, at wondered if it was related to using brio for reading and writing, so assuming UTF-8. (6666662)

There is a place in eval_with_output() where file is read assuming UTF-8 but not sure the file is UTF-8 on windows

eval_with_output <- function(code, print = FALSE, width = 80) {
path <- withr::local_tempfile()
if (!is.null(width)) {
local_width(width)
}
result <- withr::with_output_sink(path, withVisible(code))
if (result$visible && print) {
withr::with_output_sink(path, testthat_print(result$value), append = TRUE)
}
list(
val = result$value,
vis = result$visible,
out = brio::read_lines(path)
)
}

testthat:::eval_with_output(print("maëlle"))
#> $val
#> [1] "maëlle"
#> 
#> $vis
#> [1] FALSE
#> 
#> $out
#> [1] "[1] \"ma<eb>lle\""

With parent commit before brio switch

testthat::eval_with_output(print("maëlle"))
#> $val
#> [1] "maëlle"
#> 
#> $vis
#> [1] FALSE
#> 
#> $out
#> [1] "[1] \"maëlle\""

withr::with_output_sink() will let sink() write to a file. If the connection is not opened, it will probably opened a connection in native encoding and the resulting file cannot be read as UTF-8 using brio::read_lines(). This is the part that creates the "wrong" value.

I know sink() is source of issue with encoding (r-lib/evaluate#59) and file connection and encoding are not realy easy to master. So I don't know if this is something hat can be improve for windows users with snapshot test.

I just observe this

tmp <- tempfile()
sink(tmp)
print('ë')
sink()
brio::read_lines(tmp)
#> [1] "[1] \"\xeb\""
unlink(tmp)

tmp <- tempfile()
con <- file(tmp, encoding = "UTF-8")
sink(con)
print('ë')
sink()
brio::read_lines(tmp)
#> [1] "[1] \"ë\""
unlink(tmp)

Anyway, just sharing what I had found when stubbling into this issue as @maelle shared here Windows issue with me.
If possible, insuring to write as UTF-8 could help, or maybe not use brio::read_lines() in this specific place ?

Hope it helps.

@hadley hadley added bug an unexpected problem or unintended behavior snapshot 📷 labels Sep 19, 2022
@hadley hadley added this to the 3.1.5 milestone Sep 19, 2022
@hadley
Copy link
Member

hadley commented Sep 21, 2022

Is this fixed in R4.2? If so, do you need it to work in R4.1 too? (It's likely to be a couple of hours work for me, so I'd prefer to use that time on other things if your motivating issue is resolved by updating R).

@maelle
Copy link
Contributor Author

maelle commented Sep 22, 2022

Indeed it now works https://github.com/maelle/encoding.problem/actions/runs/3103119734
We can definitely work without a fix 🙂

@hadley hadley closed this as completed Sep 22, 2022
@hadley
Copy link
Member

hadley commented Sep 24, 2022

I fixed this anyway, thanks to @cderv's analysis and a hint from Kurt Hornik.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior snapshot 📷
Projects
None yet
Development

No branches or pull requests

3 participants