Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sckott opened this issue Mar 14, 2020 · 14 comments
Closed

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

sckott opened this issue Mar 14, 2020 · 14 comments
Milestone

Comments

@sckott
Copy link
Collaborator

sckott commented Mar 14, 2020

@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 sckott added this to the v0.6 milestone Mar 14, 2020
@sckott
Copy link
Collaborator Author

sckott commented Mar 14, 2020

sort of related #135

@maelle
Copy link
Member

maelle commented Mar 14, 2020

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

@sckott
Copy link
Collaborator Author

sckott commented Mar 16, 2020

hmm, maybe, will have a look

@aaronwolen
Copy link
Member

I'm not sure how to solve this problem

Yeah, this is tricky...

Would adding support for transparent compression/decompression help?

@sckott
Copy link
Collaborator Author

sckott commented Mar 17, 2020

@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
Copy link
Collaborator Author

sckott commented Mar 17, 2020

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
Copy link
Member

maelle commented Mar 21, 2020

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

@maelle
Copy link
Member

maelle commented Mar 21, 2020

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
Copy link
Member

maelle commented Mar 21, 2020

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
Copy link
Member

maelle commented Mar 21, 2020

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
Copy link
Collaborator Author

sckott commented Mar 22, 2020

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
Copy link
Collaborator Author

sckott commented Mar 22, 2020

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
Copy link
Collaborator Author

sckott commented Mar 22, 2020

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 added a commit to ropensci/rnoaa that referenced this issue Mar 23, 2020
* #290 - working on integrating vcr handling writing to disk, not done yet
related: ropensci/vcr#81 ropensci/webmockr#57

* playing with buoy tests, check back in #FIXME

* cpc_prcp test tweaks

* man file updates

* arc2 fixes, use new write to disk setup

* #290 writing to disk work, not done yet

* fixes for lcd and isd tests that also cache files

* fixed some tests #290

* fix storms test

* use relative paths with new vcr version

* move write to disk files to inst/ - test check on travis | ropensci/vcr#164

* try commenting out deleting  user cached files for ersst tests

* comment out one ersst test, does it work now?

* try ersst tests each with different file on disk - was previously using the same file
perhaps that was leading to failure if e.g. file was opened but not closed before next test ran amybe

* remove .Rinstignore, and try inst/test_files in .Rbuildignore if it works on travis

* remove inst/test_files from rbuildignore

* move test files back to tests/files/

* add .Rinstignore for ignoring test on disk files

* woops, needed to fix write_disk_path
@sckott
Copy link
Collaborator Author

sckott commented Mar 24, 2020

I think we're sorted out on this.

  • seems like tests are not included in pkg installation size
  • users can add files stored on disk associated with fixtures to .Rinstignore if needed
  • two example pkgs with files written to disk where pkg install size is good: rnoaa, crminer
  • i'll add documentation on this to the package

@sckott sckott modified the milestones: v0.6, v0.5.4 Mar 24, 2020
sckott added a commit to ropensci-books/http-testing that referenced this issue Mar 24, 2020
@sckott sckott closed this as completed in 0477396 Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants