jonkeane commented on a change in pull request #9743:
URL: https://github.com/apache/arrow/pull/9743#discussion_r606252213
##########
File path: r/tests/testthat/test-parquet.R
##########
@@ -234,3 +234,16 @@ test_that("ParquetFileReader $ReadRowGroup(s) methods", {
expect_true(reader$ReadRowGroups(c(0, 1), 0) == Table$create(x = 1:20))
expect_error(reader$ReadRowGroups(c(0, 1), 1))
})
+
+test_that("Error messages are shown when the compression algorithm lz4/snappy
+ is not found", {
+ if (codec_is_available("snappy")) {
+ d <- read_parquet(pq_file)
+ expect_is(d, "data.frame")
+ } else {
+ expect_error(
+ read_parquet(pq_file),
+ "Unsupported compressed format"
+ )
Review comment:
I don't think that the `else` branch of this test will ever be
exercised. On our minimal build test we disable (among other things) both
[snappy](https://dev.azure.com/ursacomputing/crossbow/_build/results?buildId=2724&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=6c939d89-0d1a-51f2-8b30-091a7a82e98c&l=309)
and
[parquet](https://dev.azure.com/ursacomputing/crossbow/_build/results?buildId=2724&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=6c939d89-0d1a-51f2-8b30-091a7a82e98c&l=248).
And then also note that [this entire file is skipped if parquet is not
available.](https://github.com/apache/arrow/blob/master/r/tests/testthat/test-parquet.R#L18)
Though, if this test did run in an environment where both parquet and snappy
are disabled, it shouldn't be caught and talk about compression codecs, it
should mention that parquet support was not built instead (see [my comment
above](https://github.com/apache/arrow/pull/9743/files#r605862157) about that).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
