Skip to content

Commit c3f17fa

Browse files
zero323dongjoon-hyun
authored andcommitted
[SPARK-29777][FOLLOW-UP][SPARKR] Remove no longer valid test for recursive calls
### What changes were proposed in this pull request? Disabling test for cleaning closure of recursive function. ### Why are the changes needed? As of 9514b82 this test is no longer valid, and recursive calls, even simple ones: ```lead f <- function(x) { if(x > 0) { f(x - 1) } else { x } } ``` lead to ``` Error: node stack overflow ``` This is issue is silenced when tested with `testthat` 1.x (reason unknown), but cause failures when using `testthat` 2.x (issue can be reproduced outside test context). Problem is known and tracked by [SPARK-30629](https://issues.apache.org/jira/browse/SPARK-30629) Therefore, keeping this test active doesn't make sense, as it will lead to continuous test failures, when `testthat` is updated (#27359 / SPARK-23435). ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Existing tests. CC falaki Closes #27363 from zero323/SPARK-29777-FOLLOWUP. Authored-by: zero323 <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent 40b1f4d commit c3f17fa

File tree

1 file changed

+8
-2
lines changed

1 file changed

+8
-2
lines changed

R/pkg/tests/fulltests/test_utils.R

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,10 @@ test_that("cleanClosure on R functions", {
8989
lapply(x, g) + 1 # Test for capturing function call "g"'s closure as a argument of lapply.
9090
l$field[1, 1] <- 3 # Test for access operators `$`.
9191
res <- defUse + l$field[1, ] # Test for def-use chain of "defUse", and "" symbol.
92-
f(res) # Test for recursive calls.
92+
# Enable once SPARK-30629 is fixed
93+
# nolint start
94+
# f(res) # Test for recursive calls.
95+
# nolint end
9396
}
9497
newF <- cleanClosure(f)
9598
env <- environment(newF)
@@ -101,7 +104,10 @@ test_that("cleanClosure on R functions", {
101104
# nolint end
102105
expect_true("g" %in% ls(env))
103106
expect_true("l" %in% ls(env))
104-
expect_true("f" %in% ls(env))
107+
# Enable once SPARK-30629 is fixed
108+
# nolint start
109+
# expect_true("f" %in% ls(env))
110+
# nolint end
105111
expect_equal(get("l", envir = env, inherits = FALSE), l)
106112
# "y" should be in the environment of g.
107113
newG <- get("g", envir = env, inherits = FALSE)

0 commit comments

Comments
 (0)