Implement testing for the entire repo

[caldwellst]

This is a draft PR just looking at implementing (the hardest) part of the testing, on the src/email codebase. This is also tackling a few other open issues at the same time, including:

• Consolidation of modules so that we can import functions fromt he module, not the scripts themselves (basically like using __init__.py in Python, just __init__.R in this case! Read more [here].(https://klmr.me/box/articles/box.html#nesting-modules).
• Have to work around testing {box} modules, which requires some special structure. Read more here.
• We currently streamline some slow pings to the Mailchimp API by loading the data once at the time a module is imported. For instance, see here where we load in some lists from the Mailchimp API. No longer can do that because the API is accessed when we load the module (and we have to load the modules to test!). We will move to use {memoise}. Not put in this draft PR.

The restructuring will also make things much cleaner in various modules: the indicators themselves will now have a unified structure, much different than how we do now, which will facilitate easier use and easier testing.

[zackarno]

looks cool! I assume you will let me know when it's ready for review, but will start looking at the resources you shared as links.

If I wanted to play around and manually run through some of the tests, what would be the order i'd run things

[caldwellst]

If you want to run tests, all you have to do is source the modules that have the lines:

if (is.null(box::name())) {
  box::use(dir/loc/`__tests__`
}

[caldwellst]

Otherwise, run the helper-module.R and __init__.R scripts for the relevant directory to ensure relevant modules are loaded and then you can test the tests yourself.

[zackarno]

So just to confirm the instructions:

If you want to run tests, all you have to do is source the modules that have the lines:

if (is.null(box::name())) {
  box::use(dir/loc/`__tests__`
}

here by "sourcing" the modules you don't actually mean sourcing as in source() or even box::use() like it's written in the helper-module.R function: box::use(src/email/components[...]). But rather you mean : go to the _init_.R file in the parent dir of _tests_ dir and run that file (i.e src/email/components/_init_.R)... i see this does initiate testing which looks like:

#' @export
box::use(
  src/email/components/banner_block[...],
  src/email/components/conditional_merge[...],
  src/email/components/create_template[...],
  src/email/components/email_body[...],
  src/email/components/further_info_block[...],
  src/email/components/image_block[...],
  src/email/components/intro_block[...],
  src/email/components/line_block[...],
  src/email/components/location_block[...],
  src/email/components/missing[...],
  src/email/components/summary_block[...],
  src/email/components/text_block[...]
)

if (is.null(box::name())) {
  box::use(src/email/components/`__tests__`)
}

✔ | F W  S  OK | Context
✔ |          2 | banner_block                                                                    
✖ | 1        0 | conditional_merge                                                               
─────────────────────────────────────────────────────────────────────────────────────────────────
Error (test-conditional_merge.R:5:1): (code run outside of `test_that()`)
Error in `stub(conditional_merge, "location_codes$iso3_to_regions", "Region A")`: could not find function "stub"
─────────────────────────────────────────────────────────────────────────────────────────────────
✔ |          2 | create_template                                                                 
✖ | 1        0 | email_body                                                                      
─────────────────────────────────────────────────────────────────────────────────────────────────
Error (test-email_body.R:28:3): Runs and returns single string
Error in `local_dir(here$here())`: could not find function "local_dir"
─────────────────────────────────────────────────────────────────────────────────────────────────
✔ |          2 | further_info_block                                                              
✔ |          2 | image_block                                                                     
✔ |          2 | intro_block                                                                     
✔ |          2 | line_block                                                                      
✖ | 1        0 | location_block                                                                  
─────────────────────────────────────────────────────────────────────────────────────────────────
Error (test-location_block.R:9:3): Runs and returns single string
Error in `stub(add_location, "conditional_merge$conditional_merge", function(x) x)`: could not find function "stub"
─────────────────────────────────────────────────────────────────────────────────────────────────
✔ |          5 | missing                                                                         
✔ |          2 | summary_block                                                                   
✔ |          4 | text_block                                                                      

══ Results ══════════════════════════════════════════════════════════════════════════════════════
── Failed tests ─────────────────────────────────────────────────────────────────────────────────
Error (test-conditional_merge.R:5:1): (code run outside of `test_that()`)
Error in `stub(conditional_merge, "location_codes$iso3_to_regions", "Region A")`: could not find function "stub"

Error (test-email_body.R:28:3): Runs and returns single string
Error in `local_dir(here$here())`: could not find function "local_dir"

Error (test-location_block.R:9:3): Runs and returns single string
Error in `stub(add_location, "conditional_merge$conditional_merge", function(x) x)`: could not find function "stub"

[ FAIL 3 | WARN 0 | SKIP 0 | PASS 23 ]

By this:

Otherwise, run the helper-module.R and init.R scripts for the relevant directory to ensure relevant modules are loaded and then you can test the tests yourself.

You mean we can manually run the helper-module.R file and then the _init_.R file in that same directory as the helper-module.R file and our environment will be loaded for testing so that we can walk through tests and or write additional ones. This seems to be the case however the we get the following Error w/ box::export?

> box::export()
Error in box::export() : “export” can only be called from inside a module

So i guess just skip this line for interactive dev/testing?

[caldwellst]

Sorry, didn't respond to your stuff earlier, will do now! Note that the email and utils folder are now essentially fully tested, although there's a few functions I didn't bother testing in utils.

Your errors are because for email/components the box::use() calls for {withr} and {mockery} were put ininit.Rrather thanthe helper-module.R module. I've fixed that so they should work now. The result of your explanation is basically correct. box::export() doesn't need to be run, it is there because it ensures we designate a script as a module (so it can be called by box::use(), and it only needs to be used if you don't have a #' @export call (so just in these tests/__init__.R scripts). You can get all these details form the {box} documentation on testing modules I linked above.

Otherwise, you can start reviewing some of this testing now, because it's a long and expansive amount of code @martinig94 @zackarno.

[caldwellst]

Finalized more of this, including documentation. email and utils are now fully tested. I have added in automated testing using GitHub Actions.

Good luck with review and extending the testing to the other modules!

[zackarno]

still digesting a lot of the material here, but i think the general idea for this review is to make sure there are not breaking changes, or changes that we believe are unacceptably error prone - once we check and confirm this we merge to main and deal w/ improvements/suggestions through new issues + PRs. Therefore, I'll open issues like #254 for small suggestions, questions, etc.

Also would be okay with continuing discussions here directly, but since it's a large PR we may want to try to merge it and deal w/ adjustments in smaller PRs

[zackarno]

some checklists... because i want to check boxes

• [x] run GHA dispatch on all monitor_* scripts with HS_DRY_RUN =TRUE and HS_LOCAL= TRUE
• [ ] run GHA dispatch on all monitor_* scripts with HS_DRY_RUN =TRUE and HS_LOCAL= FALSE

note this won't work on ACLED script until we merge the patch for the territory in Cyprus