Closed amoeba closed 2 weeks ago
A note about the patch here: I saw there were a number of ways to fix the issue, including just coercing everything to a vector. However, I thought it was cleaner to just directly use our internal expect_equal
helper. See:
expect_true(all(concat_int == arrow_array(1:5)))
# can be fixed by wrapping it in as.vector
expect_true(as.vector(all(concat_int == arrow_array(1:5))))
# but this seems much cleaner to me
expect_equal(concat_int, arrow_array(1:5)) # <-- this is what I went with
Can reviewers please check my assumption that expect_equal
is doing what we want here?
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit ef830524e8bb7ec6f9e74f3a4534bea8690ffc52.
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.
Rationale for this change
Update assertion code in some of the R tests to be compatible with the in-development version of testthat.
What changes are included in this PR?
Updated assertions in tests.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.
Fixes https://github.com/apache/arrow/issues/44674.