rstudio / sass

Sass compiler package for R
https://rstudio.github.io/sass/
Other
102 stars 17 forks source link

Test suite failure in connection with language setting #142

Open tillea opened 5 months ago

tillea commented 5 months ago

Describe the problem

When trying to upgrade the Debian packaged sass from version 0.4.8 to 0.4.9 I get a test suite error. We are using this test script in our CI which finally calls

LC_ALL=C.UTF-8 R --no-save < testthat.R

This results in the error

── Failure ('test-font-objects.R:131:3'): font_collection() basically works ────
`expect_collection("'foo, bar', baz", quote = FALSE, expected = "'foo, bar', baz")` threw an unexpected warning.
Message: Changing language has no effect when envvar LC_ALL='C.UTF-8'
Class:   simpleWarning/warning/condition
Backtrace:
     ▆
  1. ├─testthat::expect_warning(...) at test-font-objects.R:131:3
  2. │ └─testthat:::expect_condition_matching(...)
  3. │   └─testthat:::quasi_capture(...)
  4. │     ├─testthat (local) .capture(...)
  5. │     │ └─base::withCallingHandlers(...)
  6. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  7. └─sass (local) expect_collection("'foo, bar', baz", quote = FALSE, expected = "'foo, bar', baz")
  8.   └─testthat::expect_equal(...) at test-font-objects.R:98:3
  9.     └─testthat:::expect_waldo_equal("equal", act, exp, info, ..., tolerance = tolerance)
 10.       └─testthat:::waldo_compare(...)
 11.         └─testthat:::local_reporter_output()
 12.           └─reporter$local_user_output(.env)
 13.             └─testthat::local_reproducible_output(...)
 14.               └─withr::local_language(lang, .local_envir = .env)
 15.                 └─withr:::check_language_envvar("LC_ALL")
[ FAIL 1 | WARN 307 | SKIP 4 | PASS 188 ]

Session Info

You can find a full build log here

Please let me know if you need further information to track down the issue. In case you can't reproduce the issue the Debian Med team policy contains a hint to a docker file which could be useful to reproduce the problem on a Debian sid system. Kind regards, Andreas.

gadenbuie commented 5 months ago

Is LC_ALL=C.UTF-8 required or could you use LANG=C instead for testing? testthat sets LANG to C, which is causing this warning from withr.

This test failure points out that our check for an expected warning is a few calls away from the actual code its meant to be testing. But the failure is a false positive.

tillea commented 5 months ago

Hi,

Am Mon, May 13, 2024 at 06:40:50PM -0700 schrieb Garrick Aden-Buie:

Is LC_ALL=C.UTF-8 or could you use LANG=C instead for testing? testthat sets LANG to C, which is causing this warning from withr.

I tried this but no change:

https://salsa.debian.org/r-pkg-team/r-cran-sass/-/jobs/5724861#L367

This test failure points out that our check for an expected warning is a few calls away from the actual code its meant to be testing. But the failure is a false positive.

Sorry, I'm behind an extremely bad connection for the whole week.

Kind regards, Andreas.

gadenbuie commented 5 months ago

Instead of LC_ALL=C can you try LANG=C?

tillea commented 5 months ago

Am Tue, May 14, 2024 at 01:08:31PM -0700 schrieb Garrick Aden-Buie:

Instead of LC_ALL=C can you try LANG=C?

Works. Just uploaded. Thanks for the hint, Andreas.