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

unittest fails #502

Closed
nunotexbsd opened this issue Apr 1, 2024 · 20 comments · Fixed by #504
Closed

unittest fails #502

nunotexbsd opened this issue Apr 1, 2024 · 20 comments · Fixed by #504

Comments

@nunotexbsd
Copy link
Contributor

nunotexbsd commented Apr 1, 2024

Tests fails with same error on 2.3.5 and 2.3.6.

Any clues?

Thanks

R version 4.3.3 (2024-02-29)
Platform: amd64-portbld-freebsd14.0 (64-bit)
Running under: FreeBSD 140amd64-main 14.0-RELEASE-p5 FreeBSD 14.0-RELEASE-p5 amd64
* checking tests ...
  Running 'testthat.R'
 ERROR
Running the tests in 'tests/testthat.R' failed.
Last 13 lines of output:

  `actual$X.a4..a4..a4..e5.2` is a character vector ('<a4><a4><a4><e5>', '<a4><a4><a4><e5>', '<a4><a4><a4><e5>')
  `expected$X.a4..a4..a4..e5.2` is absent

  `actual$<a4><a4><a4><e5>1` is absent
  `expected$<a4><a4><a4><e5>1` is an integer vector (1, 2, 3)

  `actual$<a4><a4><a4><e5>2` is absent
  `expected$<a4><a4><a4><e5>2` is a character vector ('\244\244\244\345', '\244\244\244\345', '\244\244\244\345')

  [ FAIL 4 | WARN 0 | SKIP 16 | PASS 670 ]
  Error: Test failures
  In addition: Warning message:
  call dbDisconnect() when finished working with a connection
  Execution halted
* DONE

Status: 1 ERROR, 2 WARNINGs
See
  '/wrkdirs/usr/ports/databases/R-cran-RSQLite/work/RSQLite.Rcheck/00check.log'
for details.

*** Error code 1

Failed test log
testthat.Rout.fail.log

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

Thanks. What's your locale? Can you please try running in a UTF-8 locale?

@nunotexbsd
Copy link
Contributor Author

I'm running tests inside poudriere jail (packager) and root have default C.UTF-8 locale.
Tests was run with this setup.

Should I try anything else?

root@140amd64-main:~ # locale
LANG=C.UTF-8
LC_CTYPE="C.UTF-8"
LC_COLLATE="C.UTF-8"
LC_TIME="C.UTF-8"
LC_NUMERIC="C.UTF-8"
LC_MONETARY="C.UTF-8"
LC_MESSAGES="C.UTF-8"
LC_ALL=

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

Thanks. This definitely looks like an encoding issue, but it's hard for me to reproduce. Is there a Docker file/image that helps with this?

@nunotexbsd
Copy link
Contributor Author

Never used Docker but I found this link:
https://wiki.freebsd.org/Docker

Other possibility is to run a FreeBSD in a VM:
https://www.freebsd.org/where/

On my local account (outside jail), same UTF-8 locale, same error:

== Failed tests ================================================================
-- Failure ('test-encoding.R:61:3'): list the field of tables whose colnames are BIG5 encoded (#277) --
dbListFields(con, "a") (`actual`) not identical to colnames(df) (`expected`).

<snip>

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

Thanks, this is useful.

I wonder why this test is even run on your platform, it has:

  skip_on_os("linux")
  skip_on_os("mac")
  skip_on_os("solaris")

Basically, skip everywhere except on Windows. Can you please share your Sys.info() ?

@nunotexbsd
Copy link
Contributor Author

> Sys.info()
                                                                                                                                               sysname
                                                                                                                                             "FreeBSD"
                                                                                                                                               release
                                                                                                                                        "15.0-CURRENT"
                                                                                                                                               version
"FreeBSD 15.0-CURRENT #89 main-n269067-125c4560bc70: Mon Apr  1 15:39:24 WEST 2024     [email protected]:/usr/obj/usr/src/amd64.amd64/sys/GENERIC-NODEBUG"
                                                                                                                                              nodename
                                                                                                                                            "leg.home"
                                                                                                                                               machine
                                                                                                                                               "amd64"
                                                                                                                                                 login
                                                                                                                                             "nunotex"
                                                                                                                                                  user
                                                                                                                                             "nunotex"
                                                                                                                                        effective_user
                                                                                                                                             "nunotex"

@nunotexbsd
Copy link
Contributor Author

Searching for correct define for freebsd unix in https://stat.ethz.ch/R-manual/R-devel/library/base/html/Platform.html and maybe should I try skip_on_os("unix") ?

@nunotexbsd
Copy link
Contributor Author

(...)
also .Platform seems the correct way:

> .Platform
$OS.type
[1] "unix"

$file.sep
[1] "/"

$dynlib.ext
[1] ".so"

$GUI
[1] "X11"

$endian
[1] "little"

$pkgType
[1] "source"

$path.sep
[1] ":"

$r_arch
[1] ""

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

This is what is used internally:

testthat:::system_os()
#> [1] "darwin"
testthat:::system_os
#> function () 
#> tolower(Sys.info()[["sysname"]])
#> <bytecode: 0x117a85400>
#> <environment: namespace:testthat>
testthat::skip_on_os
#> function (os, arch = NULL) 
#> {
#>     os <- match.arg(os, choices = c("windows", "mac", "linux", 
#>         "solaris"), several.ok = TRUE)
#>     msg <- switch(system_os(), windows = if ("windows" %in% os) "On Windows", 
#>         darwin = if ("mac" %in% os) "On Mac", linux = if ("linux" %in% 
#>             os) "On Linux", sunos = if ("solaris" %in% os) "On Solaris")
#>     if (!is.null(arch) && !is.null(msg)) {
#>         if (!is.character(arch)) {
#>             abort("`arch` must be a character vector")
#>         }
#>         if (system_arch() %in% arch) {
#>             msg <- paste(msg, system_arch())
#>         }
#>         else {
#>             msg <- NULL
#>         }
#>     }
#>     if (is.null(msg)) {
#>         invisible(TRUE)
#>     }
#>     else {
#>         skip(msg)
#>     }
#> }
#> <bytecode: 0x117b4f190>
#> <environment: namespace:testthat>

Created on 2024-04-01 with reprex v2.1.0

So we'd either need a patch in testthat (which will take some time) or a workaround here.

Of all the patches possible, I could imagine a skip_on_os_other_than(), support for skip_on_os("bsd") or accepting the values from tolower(Sys.info()[["sysname"]]) in skip_on_os() . The workaround should likely use skip_if_not() .

@nunotexbsd
Copy link
Contributor Author

> library(testthat)
> testthat:::system_os()
[1] "freebsd"

Paching test-encoding.R:

test_that("list the field of tables whose colnames are BIG5 encoded (#277)", {
  skip_on_os("linux")
  skip_on_os("mac")
  skip_on_os("solaris")
+  skip_on_os("freebsd")

test_that("write tables whose colnames or contents are BIG5 encoded (#277)", {
  skip_on_os("linux")
  skip_on_os("mac")
  skip_on_os("solaris")
+  skip_on_os("freebsd")

test result: (reduced from 4 to 2 fails)

* checking tests ...
  Running 'testthat.R'
 ERROR
Running the tests in 'tests/testthat.R' failed.
Last 13 lines of output:
   1. \-testthat::skip_on_os("freebsd") at test-encoding.R:40:3
   2.   \-base::match.arg(...)
  -- Error ('test-encoding.R:140:3'): write tables whose colnames or contents are BIG5 encoded (#277) --
  Error in `match.arg(os, choices = c("windows", "mac", "linux", "solaris"),
      several.ok = TRUE)`: 'arg' should be one of "windows", "mac", "linux", "solaris"
  Backtrace:
      x
   1. \-testthat::skip_on_os("freebsd") at test-encoding.R:140:3
   2.   \-base::match.arg(...)

  [ FAIL 2 | WARN 0 | SKIP 16 | PASS 670 ]
  Error: Test failures
  In addition: Warning message:
  call dbDisconnect() when finished working with a connection
  Execution halted
* DONE

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

The test failures indicate that skip_on_os("bsd") doesn't work as intended.

@nunotexbsd
Copy link
Contributor Author

Error in match.arg(os, choices = c("windows", "mac", "linux", "solaris"),`

Is there a way to include freebsd in that list?

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

Yes, we'd need a patch in testthat. The alternative is a plain skip_if_not() call here, perhaps:

skip_if_not(.Platform$OS.type == "windows")

@nunotexbsd
Copy link
Contributor Author

Cool, that works fine, tests run OK and it makes some kind of logic since this test is only for windows?

test_that("list the field of tables whose colnames are BIG5 encoded (#277)", {
-  skip_on_os("linux")
-  skip_on_os("mac")
-  skip_on_os("solaris")
+  skip_if_not(.Platform$OS.type == "windows")

test_that("write tables whose colnames or contents are BIG5 encoded (#277)", {
-  skip_on_os("linux")
-  skip_on_os("mac")
-  skip_on_os("solaris")
+  skip_if_not(.Platform$OS.type == "windows")

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

Agree. I'll keep looking for the "multiple skip_on_os()" anti-pattern in the future.

@nunotexbsd
Copy link
Contributor Author

Nice, will this be fixed upstream or should I patch package manually?

I'm not the RSQLite FreeBSD port maintainer but my work depends on it for a set of new R ports.
( https://www.freshports.org/databases/R-cran-RSQLite/ )

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

Happy to review a PR here (do you mean this by "upstream"?), but it might take a while before this hits CRAN. Patching manually might be the best option right now.

@nunotexbsd
Copy link
Contributor Author

Nice. By "upstream" I mean the project coders, "downstream" will be a FreeBSD port, e.g..
Ok, I will patch it manually then.

Thanks!

@krlmlr
Copy link
Member

krlmlr commented Apr 1, 2024

I'd appreciate if you could point me to the patch or submit a PR here.

@nunotexbsd
Copy link
Contributor Author

nunotexbsd commented Apr 1, 2024

0001-Use-a-better-logic-on-tests-testthat-test-encoding.R.patch.txt

Please correct commit msg if necessary.

I think a pull request will be simplier.
Hope that it is ok.

Thanks

bpvgoncalves pushed a commit to bpvgoncalves/RSQLCipher that referenced this issue Aug 31, 2024
Under FreeBSD test fails because it only skips linux, mac and solaris.
Opimize logic and use this test only on windows.

Fixes r-dbi/RSQLite#502
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants