Skip to content

Commit adc367f

Browse files
committed
Remove improper and faulty tidyselect usage from group_by tests
- Directly "passing" a reference to a character vector object is ambiguous; we might want to test that we trigger the message about this, but this might require mucking about in the internals of tidyselect (it seems to have its own frequency mechanism separate from rlang conditions). - `tidyselect::eval_select` doesn't support `!!!`, but testthat expectations somehow makes this empty out the `group_by` arg, which _coincidentally_ passes the tests, because the default for the test archive is to group by `geo_value`. Plus, `!!!` is intended to splice/splat a list of language objects, not a character vector.
1 parent e26f6dd commit adc367f

File tree

1 file changed

+7
-24
lines changed

1 file changed

+7
-24
lines changed

tests/testthat/test-methods-epi_archive.R

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,17 @@ test_that("quosure passing issue in epix_slide is resolved + other potential iss
8282
),
8383
reference
8484
)
85-
# test the passing-string-var behavior:
85+
# Might also want to test the passing-string-var-without-all_of behavior, but
86+
# make sure to set, trigger, then reset (or restore to old value) the
87+
# tidyselect once-per-session message about the ambiguity
88+
#
89+
# test the passing-all-of-string-var behavior:
8690
my_group_by = "geo_value"
8791
expect_identical(
8892
epix_slide(x = archive_cases_dv_subset,
8993
f = ~ mean(.x$case_rate_7d_av),
9094
n = 3,
91-
group_by = my_group_by,
95+
group_by = tidyselect::all_of(my_group_by),
9296
ref_time_values = time_values,
9397
new_col_name = 'case_rate_3d_av'),
9498
reference
@@ -97,28 +101,7 @@ test_that("quosure passing issue in epix_slide is resolved + other potential iss
97101
archive_cases_dv_subset$slide(
98102
f = ~ mean(.x$case_rate_7d_av),
99103
n = 3,
100-
group_by = my_group_by,
101-
ref_time_values = time_values,
102-
new_col_name = 'case_rate_3d_av'
103-
),
104-
reference
105-
)
106-
# test the passing-splatted-string-var behavior:
107-
my_group_by = "geo_value"
108-
expect_identical(
109-
epix_slide(x = archive_cases_dv_subset,
110-
f = ~ mean(.x$case_rate_7d_av),
111-
n = 3,
112-
group_by = !!!my_group_by,
113-
ref_time_values = time_values,
114-
new_col_name = 'case_rate_3d_av'),
115-
reference
116-
)
117-
expect_identical(
118-
archive_cases_dv_subset$slide(
119-
f = ~ mean(.x$case_rate_7d_av),
120-
n = 3,
121-
group_by = !!!my_group_by,
104+
group_by = tidyselect::all_of(my_group_by),
122105
ref_time_values = time_values,
123106
new_col_name = 'case_rate_3d_av'
124107
),

0 commit comments

Comments
 (0)