Tests for knitting all report types, via enabling non-interactive use of create_visc_project(), use_visc_report(), and use_visc_methods()

slager commented 3 months ago

This PR sets up an automated unit test for successfully knitting a generic report PDF across all platforms. This will help with future unit test writing, and gets us a lot farther along on #136. The test is also the reprex for #159, so it will greatly help me iterate on debugging that on statsrv once I merge these changes into the #159 branch.

The CI runs on this PR are failing, but for a very good reason: They're successfully catching the 'Lonely item' latex bug across all platforms. See #169 (but don't merge it!) for a demonstration that once I combine these with Kellie's bugfix, the generic reports successfully knit on all the CI platforms.

Along the way, I had to make a few changes to a few functions to enable non-interactive use (see https://github.com/FredHutch/VISCtemplates/issues/136#issuecomment-2176681161).

Changes in this PR:

Exported functions create_visc_project(), use_visc_report(), and use_visc_methods() now have an additional argument interactive that is always TRUE by default, with the same behavior and UI as before. Setting interactive = FALSE produces the same results as before, but without any interactive side effects like automatic Rstudio window/project switching, interactive user confirmations, interactive user questions, or console outputs. The new arguments have been documented for the 3 functions.
Internal helper functions challenge_visc_name(), challenge_directory(), and challenge_visc_report() also now have an interactive option that defaults to TRUE but can be set to FALSE with the same effect. These helpers are called from within the 3 exported functions above.
Replaced use of the here package with rprojroot to facilitate unit testing. here wasn't flexible enough to handle test knitting an Rmd file in a project package from within the VISCtemplates unit tests.
CI tweaks to facilitate installing dependencies and building the report on the R CMD check runners
Automated unit test for knitting a generic report

slager commented 3 months ago

This is ready for review now!

kelliemac commented 3 months ago

This looks great!! Thanks so much for doing this, Dave!! I added a few comments to tests/testthat/test-template.R about ways to make the testing a little more comprehensive. I think we may as well try to tackle these now and hopefully it won't be too much more work, but if there are any roadblocks let us know and we can push those off for later.

slager commented 3 months ago

Thanks! I had already started working locally on knitting the empty report also (no problems!) and the word versions, so will add those, will try adding bama/nab, and will see if I can get the snapshot artifact upload to work as well.

kelliemac commented 3 months ago

great! I'll keep an eye out for updates.

kelliemac commented 3 months ago

This is looking great!! For example, I can download the test snapshot artifacts generated in draft PR #169 by going to https://github.com/FredHutch/VISCtemplates/actions/runs/9701586982/job/26775548607?pr=169#step:8:394 and downloading and then reviewing the pdf and docx files. Yay!

slager commented 3 months ago

Yup! It's almost there, and the snapshots are getting uploaded in #169 like you say! The only thing I'm still looking into is having just the snapshots uploaded in the artifact, not the whole results tree when tests fail like what's happening now in this branch https://github.com/FredHutch/VISCtemplates/actions/runs/9701659385/job/26775775183?pr=168. There's either something I'm not fully understanding about the r-lib/actions yaml or a bug in how that works when tests fail. Hopefully more TBA soon, thanks for approving but let's hold off on merging for now.

kelliemac commented 3 months ago

sounds good!!

slager commented 3 months ago

I'm pretty convinced now that this is happening because of the upstream yml. We'll see if they're amenable to a PR that would let us have the flexibility to always upload snapshots, even when tests fail. They already have that functionality built into the results artifact upload, just not for snapshots.

https://github.com/r-lib/actions/issues/871#issuecomment-2195460751

kelliemac commented 3 months ago

sounds good, Dave! is it worth merging this PR while we wait on the r-lib folks? or is there anything you want to wait on?

slager commented 3 months ago

Yep! I'll add a few more code comments today, then merge it, then add an issue/reminder for tweaking the CI if/when upstream changes their yml.

slager commented 3 months ago

Leaving the branch here for now in case I need to refer to it during yml discussion with r-lib/actions upstream.

slager commented 2 months ago

Deleting branch since there hasn't been movement on this from upstream. Will resurrect later as needed.

FredHutch / VISCtemplates

Tests for knitting all report types, via enabling non-interactive use of create_visc_project(), use_visc_report(), and use_visc_methods() #168