[WIP][SPARK-23435][SPARKR][TESTS] Update testthat to >= 2.0.0

[zero323]

What changes were proposed in this pull request?

• Update testthat to >= 2.0.0
• Replace of testthat:::run_tests with testthat:::test_package_dir
• Add trivial assertions for tests, without any expectations, to avoid skipping.
• Update related docs.

Why are the changes needed?

testthat version has been frozen by SPARK-22817 / #20003, but 1.0.2 is pretty old, and we shouldn't keep things in this state forever.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

Note

This change leads to

Error: Test failed: 'cleanClosure on R functions'

• node stack overflow

in test_utils cleanClosure test, which requires further investigation.

CC @HyukjinKwon

[SparkQA]

Test build #117261 has finished for PR 27328 at commit ac6d524.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

cc @felixcheung

[HyukjinKwon]

If it works, we should install the latest testthat in Jenkins machine. cc @shaneknapp

[HyukjinKwon]

@zero323, thanks for putting your efforts here. As long as the tests and CRAN check pass, it should be good to go.

[zero323]

Hmmm... So it looks like it actually yields some additional errors on Windows.

test_sparkSQL.R:298: skip: create DataFrame from RDD
Reason: Hive is not build with SparkSQL, skipped
test_sparkSQL.R:862: failure: collect() support Unicode characters
rdf$name[1] not equal to markUtf8("<U+C548><U+B155><U+D558><U+C138><U+C694>").
1/1 mismatches
x[1]: "<U+C548><U+B155><U+D558><U+C138><U+C694>"
y[1]: "<U+C548><U+B155><U+D558><U+C138><U+C694>"
test_sparkSQL.R:863: failure: collect() support Unicode characters
rdf$name[2] not equal to markUtf8("<U+60A8><U+597D>").
1/1 mismatches
x[1]: "<U+60A8><U+597D>"
y[1]: "<U+60A8><U+597D>"
test_sparkSQL.R:864: failure: collect() support Unicode characters
rdf$name[3] not equal to markUtf8("<U+3053><U+3093><U+306B><U+3061><U+306F>").
1/1 mismatches
x[1]: "<U+3053><U+3093><U+306B><U+3061><U+306F>"
y[1]: "<U+3053><U+3093><U+306B><U+3061><U+306F>"
test_sparkSQL.R:865: failure: collect() support Unicode characters
rdf$name[4] not equal to markUtf8("Xin ch�o").
1/1 mismatches
x[1]: NA
y[1]: "Xin ch�o"
test_sparkSQL.R:868: failure: collect() support Unicode characters
collect(where(df1, df1$name == markUtf8("<U+60A8><U+597D>")))$name not equal to markUtf8("<U+60A8><U+597D>").
Modes: list, character
Length mismatch: comparison on first 0 components
test_sparkSQL.R:1325: skip: test HiveContext
Reason: Hive is not build with SparkSQL, skipped

Especially the last two are worrying. I'll investigate once I put my hands on some Windows environment.

[SparkQA]

Test build #117325 has finished for PR 27328 at commit 8215126.

• This patch fails R style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117328 has finished for PR 27328 at commit c2a5157.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117330 has finished for PR 27328 at commit 2e785d3.

• This patch fails R style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[zero323]

I've checked some of the failures, and it is pretty clear that these tests must have been silenced somehow.

• node stack overflow in cleanClosure is an obvious bug and it is trivial to reproduce outside tests - I've opened SPARK-30629 to track this one.

• Seems like mismatches in collect() support Unicode characters are caused by Windows / R encoding quirks. As is right now lines don't even seem to be recognized as a valid JSON input (setting UTF-8 directly on lines helps with that, but doesn't resolve the whole problem).

  library(SparkR)
  SparkR::sparkR.session()
  Sys.info()
  #           sysname           release           version 
  #         "Windows"      "Server x64"     "build 17763" 
  #          nodename           machine             login 
  # "WIN-5BLT6Q610KH"          "x86-64"   "Administrator" 
  #              user    effective_user 
  #   "Administrator"   "Administrator" 
  
  Sys.getlocale()
  
  # [1] "LC_COLLATE=English_United States.1252;LC_CTYPE=English_United States.1252;LC_MONETARY=English_United States.1252;LC_NUMERIC=C;LC_TIME=English_United States.1252"
  
  lines <- c("{\"name\":\"안녕하세요\"}",
             "{\"name\":\"您好\", \"age\":30}",
             "{\"name\":\"こんにちは\", \"age\":19}",
             "{\"name\":\"Xin chào\"}")
  
  system(paste0("cat ", jsonPath))
  # {"name":"<U+C548><U+B155><U+D558><U+C138><U+C694>"}
  # {"name":"<U+60A8><U+597D>", "age":30}
  # {"name":"<U+3053><U+3093><U+306B><U+3061><U+306F>", "age":19}
  # {"name":"Xin chào"}
  # [1] 0
  
  
  jsonPath <- tempfile(pattern = "sparkr-test", fileext = ".tmp")
  writeLines(lines, jsonPath)
  
  df <- read.df(jsonPath, "json")
  
  
  printSchema(df)
  # root
  #  |-- _corrupt_record: string (nullable = true)
  #  |-- age: long (nullable = true)
  #  |-- name: string (nullable = true)
  
  head(df)
  #              _corrupt_record age                                     name
  # 1                       <NA>  NA <U+C548><U+B155><U+D558><U+C138><U+C694>
  # 2                       <NA>  30                         <U+60A8><U+597D>
  # 3                       <NA>  19 <U+3053><U+3093><U+306B><U+3061><U+306F>
  # 4 {"name":"Xin ch<U+FFFD>o"}  NA                                     <NA>

Let's see what else surfaces downstream.

[SparkQA]

Test build #117332 has finished for PR 27328 at commit fd9ec4e.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117355 has finished for PR 27328 at commit 29abbd8.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117358 has finished for PR 27328 at commit 3bb15aa.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117359 has finished for PR 27328 at commit 5468b4b.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117367 has finished for PR 27328 at commit e02d85e.

• This patch fails R style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[shaneknapp]

NOOOOOOOOOOOO!!!!!!!!11111one

i really don't want to touch the extremely fragile R "ecosystem" unless i have to. but, this looks like i probably should... :(

i'll experiment w/upgrading testthat to 2.0.0 on my staging server and cross my fingers that it doesn't open dependency hell.

we also might need to backport this change for the 2.4 branch if it causes those builds to fail.

created a sub-task in jira for this: https://issues.apache.org/jira/browse/SPARK-30637

[shaneknapp]

welp, uninstalling 1.0.2 and reinstalling 2.0.0 on my staging worker went smoothly... which makes me nervous lol. :)

* installing *source* package ‘testthat’ ...
** package ‘testthat’ successfully unpacked and MD5 sums checked
** libs
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG -I../inst/include -DCOMPILING_TESTTHAT     -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c init.c -o init.o
gcc -std=gnu99 -I/usr/share/R/include -DNDEBUG -I../inst/include -DCOMPILING_TESTTHAT     -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c reassign.c -o reassign.o
g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -DCOMPILING_TESTTHAT     -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c test-catch.cpp -o test-catch.o
g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -DCOMPILING_TESTTHAT     -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c test-example.cpp -o test-example.o
g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -DCOMPILING_TESTTHAT     -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c test-runner.cpp -o test-runner.o
g++ -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -Wl,-z,relro -o testthat.so init.o reassign.o test-catch.o test-example.o test-runner.o -L/usr/lib/R/lib -lR
installing to /usr/local/lib/R/site-library/testthat/libs
** R
** inst
** tests
** preparing package for lazy loading
** help
*** installing help indices
*** copying figures
** building package indices
** installing vignettes
** testing if installed package can be loaded
* DONE (testthat)

[SparkQA]

Test build #117371 has finished for PR 27328 at commit 0265727.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117374 has finished for PR 27328 at commit abcaf3d.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #117376 has finished for PR 27328 at commit 684831b.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[zero323]

@shaneknapp

NOOOOOOOOOOOO!!!!!!!!11111one

i really don't want to touch the extremely fragile R "ecosystem" unless i have to. but, this looks like i probably should... :(

If you don't mind me asking ‒ have you thought about moving R environments to ananconda. If I recall correctly some discussions on dev, you we're pretty enthusiastic about using it with Python.

[HyukjinKwon]

@shaneknapp, I know what's like (#27328 (comment)) .. lol .. this has been the pain in the ass in the dev env because nowadays many R libraries have a higher version of testthat as a dependency

[shaneknapp]

@shaneknapp

NOOOOOOOOOOOO!!!!!!!!11111one

i really don't want to touch the extremely fragile R "ecosystem" unless i have to. but, this looks like i probably should... :(

If you don't mind me asking ‒ have you thought about moving R environments to ananconda. If I recall correctly some discussions on dev, you we're pretty enthusiastic about using it with Python.

i just tried moving to anaconda and it doesn't seem to support versions high enough of many of the packages that we require (for example, testthat is 1.0.7 or something IIRC).

[zero323]

@shaneknapp

i just tried moving to anaconda and it doesn't seem to support versions high enough of many of the packages that we require (for example, testthat is 1.0.7 or something IIRC).

2.3.1 is available from forge, and if nothing changed, the remaining ones are as well. I should have working environment files stashed somewhere, if that helps, but there probably in some place I won't be able to access for a week or two.

[shaneknapp]

a week of two is fine, and i'm more than happy to play w/an environment file and see if that works.

…

On Fri, Feb 28, 2020 at 12:46 PM Maciej ***@***.***> wrote: @shaneknapp <https://github.com/shaneknapp> i just tried moving to anaconda and it doesn't seem to support versions high enough of many of the packages that we require (for example, testthat is 1.0.7 or something IIRC). 2.3.1 is available <https://github.com/conda-forge/r-testthat-feedstock> from forge, and if nothing changed, the remaining ones are as well. I should have working environment files stashed somewhere, if that helps, but there probably in some place I won't be able to access for a week or two. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27328?email_source=notifications&email_token=AAMIHLFM2YJV2U6PBNNOTHTRFFZ2HA5CNFSM4KKOGNIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENKDGWA#issuecomment-592720728>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMIHLEYO4EBWKDDS7HVIGTRFFZ2HANCNFSM4KKOGNIA> .

[shaneknapp]

actually, it seems that i've gotten the R suite of drama packages installed to the latest/greatest 3.6.3 via apt, and all the required libs (including testthat @ 2.3.2) installed via install.packages, and IT WORKS.

these are on ubuntu 18.04LTS dev workers, but i'm about to release them to the general ubuntu worker pool and reimage the old ubuntu 16 workers w/18. YAY!