Skip to content

Commit b2bde0e

Browse files
committed
Make epix_slide tidyselect tests tougher (not just default grping)
Existing tests of the resolution of some rlang&tidyselect issues in epix_slide were a bit loose, testing only different ways of explicitly specifying a grouping that all were equal to the default grouping. This came into play with a test that contained faulty usage of `!!!` which appeared to be somehow transformed by testthat expectations into something zapping the `group_by` arg from the related call. This tougher line of testing is intended to catch any similar types of issues with the other test cases.
1 parent adc367f commit b2bde0e

File tree

1 file changed

+37
-24
lines changed

1 file changed

+37
-24
lines changed

tests/testthat/test-methods-epi_archive.R

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -45,84 +45,97 @@ test_that("quosure passing issue in epix_slide is resolved + other potential iss
4545
time_values <- seq(as.Date("2020-06-01"),
4646
as.Date("2020-06-02"),
4747
by = "1 day")
48-
reference = epix_slide(x = archive_cases_dv_subset,
49-
f = ~ mean(.x$case_rate_7d_av),
50-
n = 3,
51-
group_by = geo_value,
52-
ref_time_values = time_values,
53-
new_col_name = 'case_rate_3d_av')
48+
# We only have one non-version, non-time key in the example archive. Add
49+
# another so that we don't accidentally pass tests due to accidentally
50+
# matching the default grouping.
51+
ea = as_epi_archive(archive_cases_dv_subset$DT %>%
52+
dplyr::mutate(modulus = seq_len(nrow(.)) %% 5L),
53+
other_keys = "modulus",
54+
compactify = TRUE)
55+
reference_by_modulus = epix_slide(x = ea,
56+
f = ~ mean(.x$case_rate_7d_av),
57+
n = 3,
58+
group_by = modulus,
59+
ref_time_values = time_values,
60+
new_col_name = 'case_rate_3d_av')
61+
reference_by_both = epix_slide(x = ea,
62+
f = ~ mean(.x$case_rate_7d_av),
63+
n = 3,
64+
group_by = c(geo_value, modulus),
65+
ref_time_values = time_values,
66+
new_col_name = 'case_rate_3d_av')
5467
# test the passing-something-that-must-be-enquosed behavior:
5568
expect_identical(
56-
archive_cases_dv_subset$slide(
69+
ea$slide(
5770
f = ~ mean(.x$case_rate_7d_av),
5871
n = 3,
59-
group_by = geo_value,
72+
group_by = modulus,
6073
ref_time_values = time_values,
6174
new_col_name = 'case_rate_3d_av'
6275
),
63-
reference
76+
reference_by_modulus
6477
)
6578
# test the passing-string-literal behavior:
6679
expect_identical(
67-
epix_slide(x = archive_cases_dv_subset,
80+
epix_slide(x = ea,
6881
f = ~ mean(.x$case_rate_7d_av),
6982
n = 3,
70-
group_by = "geo_value",
83+
group_by = "modulus",
7184
ref_time_values = time_values,
7285
new_col_name = 'case_rate_3d_av'),
73-
reference
86+
reference_by_modulus
7487
)
7588
expect_identical(
76-
archive_cases_dv_subset$slide(
89+
ea$slide(
7790
f = ~ mean(.x$case_rate_7d_av),
7891
n = 3,
79-
group_by = "geo_value",
92+
group_by = "modulus",
8093
ref_time_values = time_values,
8194
new_col_name = 'case_rate_3d_av'
8295
),
83-
reference
96+
reference_by_modulus
8497
)
8598
# Might also want to test the passing-string-var-without-all_of behavior, but
8699
# make sure to set, trigger, then reset (or restore to old value) the
87100
# tidyselect once-per-session message about the ambiguity
88101
#
89102
# test the passing-all-of-string-var behavior:
90-
my_group_by = "geo_value"
103+
my_group_by = "modulus"
91104
expect_identical(
92-
epix_slide(x = archive_cases_dv_subset,
105+
epix_slide(x = ea,
93106
f = ~ mean(.x$case_rate_7d_av),
94107
n = 3,
95108
group_by = tidyselect::all_of(my_group_by),
96109
ref_time_values = time_values,
97110
new_col_name = 'case_rate_3d_av'),
98-
reference
111+
reference_by_modulus
99112
)
100113
expect_identical(
101-
archive_cases_dv_subset$slide(
114+
ea$slide(
102115
f = ~ mean(.x$case_rate_7d_av),
103116
n = 3,
104117
group_by = tidyselect::all_of(my_group_by),
105118
ref_time_values = time_values,
106119
new_col_name = 'case_rate_3d_av'
107120
),
108-
reference
121+
reference_by_modulus
109122
)
110123
# test the default behavior (default in this case should just be "geo_value"):
111124
expect_identical(
112-
epix_slide(x = archive_cases_dv_subset,
125+
epix_slide(x = ea,
113126
f = ~ mean(.x$case_rate_7d_av),
114127
n = 3,
115128
ref_time_values = time_values,
116129
new_col_name = 'case_rate_3d_av'),
117-
reference
130+
reference_by_both
118131
)
119132
expect_identical(
120-
archive_cases_dv_subset$slide(
133+
ea$slide(
121134
f = ~ mean(.x$case_rate_7d_av),
122135
n = 3,
123136
ref_time_values = time_values,
124137
new_col_name = 'case_rate_3d_av'
125138
),
126-
reference
139+
reference_by_both
127140
)
128141
})

0 commit comments

Comments
 (0)