Skip to content

Commit 27d1250

Browse files
Copilotm7pr
andcommitted
Update tests to use public API instead of internal functions - create qenv-based tests for function call dependency fix
Co-authored-by: m7pr <[email protected]>
1 parent a5fdb4c commit 27d1250

File tree

1 file changed

+104
-76
lines changed

1 file changed

+104
-76
lines changed
Lines changed: 104 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,89 +1,117 @@
1-
testthat::test_that("function calls should not appear on left side of assignment in dependencies", {
2-
# Test for the specific issue reported in GitHub issue #262
1+
testthat::test_that("get_code correctly handles function calls in assignments without false dependencies", {
2+
# Test reproducing the issue from GitHub #262 using iris dataset
3+
# Function calls on left side of assignments should not create false dependencies
34

4-
# Create a minimal example that reproduces the problem
5-
code1 <- 'ADMH[c("test")] <- c("value")'
5+
data_env <- qenv() |>
6+
within({
7+
# Create initial datasets
8+
ADSL <- iris
9+
ADMH <- mtcars
10+
ADVS <- iris
11+
12+
# This assignment uses function calls on the left side
13+
# colnames(ADMH[c("mpg", "cyl")]) should not create dependency for ADVS
14+
colnames(ADMH[c("mpg", "cyl")]) <- c("Miles_Per_Gallon", "Cylinders")
15+
16+
# ADVS should be independent of ADMH modifications
17+
ADVS <- cbind(ADVS, Species_Number = as.numeric(ADVS$Species))
18+
})
619

7-
# Parse and extract dependencies
8-
parsed1 <- parse(text = code1, keep.source = TRUE)
20+
# Get code for ADVS - should NOT include the ADMH colnames modification
21+
advs_code <- get_code(data_env, names = "ADVS")
922

10-
# Extract dependencies using the internal function
11-
deps1 <- teal.code:::extract_dependency(parsed1)
23+
# ADVS code should not include the ADMH modification line
24+
testthat::expect_false(grepl("colnames\\\\(ADMH", advs_code)))
1225

13-
# The issue: function 'c' should not appear on left side of assignment
14-
# In deps1, 'c' appears before '<-', which is incorrect
15-
assign_pos1 <- which(deps1 == "<-")
16-
left_side1 <- if(length(assign_pos1) > 0) deps1[seq_len(assign_pos1[1] - 1)] else character(0)
26+
# ADVS code should include its own definition and dependency on initial ADVS
27+
testthat::expect_true(grepl("ADVS <- iris", advs_code))
1728

18-
# Function calls should NOT appear on the left side of assignment
19-
testthat::expect_false("c" %in% left_side1,
20-
info = "Function 'c' should not appear on left side of assignment in dependencies")
29+
testthat::expect_true(grepl("ADVS <- cbind", advs_code))
30+
})
31+
32+
testthat::test_that("get_code correctly excludes unrelated function call assignments", {
33+
# Test that function calls like names(), class(), attr() don't create false dependencies
2134

22-
# Only actual variables/objects should appear on left side
23-
# In this case, only 'ADMH' should be on the left side
24-
testthat::expect_true("ADMH" %in% left_side1,
25-
info = "Variable 'ADMH' should appear on left side as it's being assigned to")
35+
data_env <- qenv() |>
36+
within({
37+
dataset_a <- iris[1:10, ]
38+
dataset_b <- mtcars[1:5, ]
39+
40+
# Modify dataset_a attributes using function calls on left side
41+
names(dataset_a[c("Sepal.Length", "Sepal.Width")]) <- c("SL", "SW")
42+
class(dataset_a) <- c("custom_iris", class(dataset_a))
43+
44+
# dataset_b should be independent of dataset_a modifications
45+
dataset_b$new_column <- dataset_b$mpg * 2
46+
})
2647

27-
# Function calls should still appear as dependencies on the right side
28-
right_side1 <- if(length(assign_pos1) > 0) deps1[seq(assign_pos1[1] + 1, length(deps1))] else character(0)
29-
testthat::expect_true("c" %in% right_side1,
30-
info = "Function 'c' should appear on right side as a dependency")
48+
# Get code for dataset_b - should NOT include dataset_a modifications
49+
dataset_b_code <- get_code(data_env, names = "dataset_b")
50+
51+
# Check that dataset_b code doesn't include dataset_a function call modifications
52+
testthat::expect_false(grepl("names\\\\(dataset_a", dataset_b_code)))
53+
testthat::expect_false(grepl("class\\\\(dataset_a", dataset_b_code)))
54+
55+
# But should include its own definition and modifications
56+
testthat::expect_true(grepl("dataset_b <- mtcars", dataset_b_code))
57+
testthat::expect_true(grepl("dataset_b\\$new_column", dataset_b_code))
3158
})
3259

33-
testthat::test_that("complex assignment with function calls handles dependencies correctly", {
34-
# This test reproduces the exact scenario from the GitHub issue
35-
36-
code_admh <- 'teal.data::col_labels(ADMH[c("MHDISTAT")]) <- c("Status of Disease")'
37-
38-
parsed_admh <- parse(text = code_admh, keep.source = TRUE)
39-
40-
deps_admh <- teal.code:::extract_dependency(parsed_admh)
41-
42-
# Check ADMH dependencies
43-
assign_pos_admh <- which(deps_admh == "<-")
44-
left_side_admh <- if(length(assign_pos_admh) > 0) deps_admh[seq_len(assign_pos_admh[1] - 1)] else character(0)
45-
right_side_admh <- if(length(assign_pos_admh) > 0) deps_admh[seq(assign_pos_admh[1] + 1, length(deps_admh))] else character(0)
46-
47-
# Function calls should not be on left side of assignment
48-
testthat::expect_false("c" %in% left_side_admh,
49-
info = "Function 'c' should not be on left side in ADMH dependencies")
50-
testthat::expect_false("col_labels" %in% left_side_admh,
51-
info = "Function 'col_labels' should not be on left side in ADMH dependencies")
52-
53-
# Functions can be on right side as dependencies
54-
testthat::expect_true("c" %in% right_side_admh,
55-
info = "Function 'c' can appear on right side as a dependency")
56-
testthat::expect_true("col_labels" %in% right_side_admh,
57-
info = "Function 'col_labels' should be moved to right side as a dependency")
58-
59-
# Variables being modified should be on left side
60-
testthat::expect_true("ADMH" %in% left_side_admh,
61-
info = "Variable 'ADMH' should be on left side as it's being modified")
60+
testthat::test_that("get_code handles complex function calls without creating circular dependencies", {
61+
# Test complex scenarios with nested function calls similar to the original issue
62+
63+
data_env <- qenv() |>
64+
within({
65+
base_data <- iris
66+
processed_data <- mtcars
67+
final_data <- iris[1:5, ]
68+
69+
# Complex assignment with nested function calls - should not affect final_data
70+
attr(processed_data[c("mpg", "hp")], "custom_attr") <- list(source = "mtcars", type = "numeric")
71+
72+
# Another complex assignment with function calls
73+
levels(base_data$Species)[c(1, 2)] <- c("Type1", "Type2")
74+
75+
# final_data should be independent of the above modifications
76+
final_data <- transform(final_data, Sepal.Sum = Sepal.Length + Sepal.Width)
77+
})
78+
79+
# Get code for final_data
80+
final_data_code <- get_code(data_env, names = "final_data")
81+
82+
# final_data should not include the complex function call assignments from other datasets
83+
testthat::expect_false(grepl("attr\\\\(processed_data", final_data_code)))
84+
testthat::expect_false(grepl("levels\\\\(base_data", final_data_code)))
85+
86+
# But should include its own operations
87+
testthat::expect_true(grepl("final_data <- iris", final_data_code))
88+
testthat::expect_true(grepl("transform\\\\(final_data", final_data_code)))
6289
})
6390

64-
testthat::test_that("function calls in complex expressions are handled correctly", {
65-
# Test various complex expressions to ensure function calls don't appear on left side
66-
67-
test_cases <- list(
68-
'result <- fun(a, b)',
69-
'data[filter(x)] <- transform(y)',
70-
'obj$method(params) <- compute(values)'
71-
)
72-
73-
for (test_code in test_cases) {
74-
parsed_code <- parse(text = test_code, keep.source = TRUE)
75-
deps <- teal.code:::extract_dependency(parsed_code)
76-
77-
assign_pos <- which(deps == "<-")
78-
if (length(assign_pos) > 0) {
79-
left_side <- deps[seq_len(assign_pos[1] - 1)]
91+
testthat::test_that("get_code preserves function dependencies while avoiding false assignment targets", {
92+
# Test that functions are still tracked as dependencies but not as assignment targets
93+
94+
data_env <- qenv() |>
95+
within({
96+
my_data <- iris
97+
helper_func <- function(x, cols) names(x)[cols]
8098

81-
# No function calls should appear on left side
82-
function_names <- c("fun", "filter", "transform", "method", "compute")
83-
for (func_name in function_names) {
84-
testthat::expect_false(func_name %in% left_side,
85-
info = paste("Function", func_name, "should not appear on left side in:", test_code))
86-
}
87-
}
88-
}
99+
# Assignment that should depend on helper_func but not treat it as assignment target
100+
names(my_data[c(1, 2)]) <- helper_func(my_data, c(1, 2))
101+
102+
# Create another object that uses my_data
103+
summary_data <- summary(my_data)
104+
})
105+
106+
# Get code for summary_data - should include helper_func definition due to dependency
107+
summary_code <- get_code(data_env, names = "summary_data")
108+
109+
# Should include helper_func since it's used in modifying my_data
110+
testthat::expect_true(grepl("helper_func <- function", summary_code))
111+
112+
# Should include the names assignment that uses helper_func
113+
testthat::expect_true(grepl("names\\\\(my_data", summary_code)))
114+
115+
# Should include my_data initial definition
116+
testthat::expect_true(grepl("my_data <- iris", summary_code))
89117
})

0 commit comments

Comments
 (0)