ropensci / vcr

Record and replay HTTP requests
https://docs.ropensci.org/vcr
Other
77 stars 12 forks source link

large files from writing to disk can make a package build too large #164

Closed sckott closed 4 years ago

sckott commented 4 years ago

@maelle @aaronwolen need your help on this.

So with the support for writing to disk in vcr, the files written to disk especially can be quite large. Fixtures can be large too, but since they're plain text it seems like they generally are pretty small.

Lets say the files recorded to disk are in tests/files. If you add tests/files to .Rbuildignore tests run fine if you run them with devtools::test(), but NOT if you run R CMD CHECK or equivalents.

You can not add tests/files to .Rbuildignore - and then R CMD CHECK works - BUT then when you build the package, the files in tests/files are included - and if they are very large, the build can be too large for CRAN's 5 MB max.

I'm not sure how to solve this problem

sckott commented 4 years ago

sort of related https://github.com/ropensci/vcr/issues/135

maelle commented 4 years ago

Would it work for r cmd check to use inst/ and .Rinstignore? Probably not?

sckott commented 4 years ago

hmm, maybe, will have a look

aaronwolen commented 4 years ago

I'm not sure how to solve this problem

Yeah, this is tricky...

Would adding support for transparent compression/decompression help?

sckott commented 4 years ago

@aaronwolen Thats a good idea. Have not tried it yet, so can't comment on it, but does seem like a harder route potentially.

so the commit above in the rnoaa package, I tried @maelle 's approach putting files in inst/test_files/ instead of in tests/ - and that does seem to work in that we can still run tests with http requests that write files to disk & the on disk files don't increase the package build size

There's definitely lots more edge cases to work out to make sure this is a general solution.

putting inst/test_files/ in .Rinstignore doesn't have any effect on package build size, but putting inst/test_files/ in .Rbuildignore does change package build size

if either of you have http requests that write to disk and can test this out that'd be super helpful.

sckott commented 4 years ago

the checks for rnoaa for the above setup is mostly working https://travis-ci.org/github/ropensci/rnoaa/jobs/663700897 - i think the failing test is unrelated but still checking into it

maelle commented 4 years ago

For CRAN, tests will probably need to be turned off anyway?

maelle commented 4 years ago

I'm about to submit a new version of ropenaq to CRAN and it's quite big because some fixtures are very big. I'm not sure yet how I'll handle that.

maelle commented 4 years ago

One thing is that I probably don't need the fixtures to be that large but to make them smaller I'll need to edit them by hand. Not sure how much that applies to large files written to disk i.e. can one make them smaller by hand/make fake files instead, without too much hassle.

maelle commented 4 years ago

btw does the size limit apply to tests?

"Packages should be of the minimum necessary size. Reasonable compression should be used for data (not just .rda files) and PDF documentation: CRAN will if necessary pass the latter through qpdf.

As a general rule, neither data nor documentation should exceed 5MB (which covers several books). A CRAN package is not an appropriate way to distribute course notes, and authors will be asked to trim their documentation to a maximum of 5MB. "

https://cran.r-project.org/web/packages/policies.html

sckott commented 4 years ago

For CRAN, tests will probably need to be turned off anyway?

depends i think, if the test files don't make the package too big then they could be run

sckott commented 4 years ago

can one make them smaller by hand/make fake files instead, without too much hassle.

i guess that would be very case dependent - and tricky: to make sure the hand modified fixture will still work with the rest of the package code

sckott commented 4 years ago

does the size limit apply to tests?

I don't know actually, all I was looking at was the file size after installing with various configurations.

sckott commented 4 years ago

I think we're sorted out on this.