@meetagrawal09 Can you comment in general on whether my mocking approach makes sense here, and specifically check my reasoning for going against the testthat recommendations?

The testthat docs advise not mocking objects from other namespaces because [my paraphrasings]: (1) it'll affect anything else that calls into that namespace during the life of the mock, even if from outside the code under test, and (2) the point of namespaces is to be sealed, dangit. They recommend instead to either import the functions in question or to encapsulate them in wrapper functions and then mock those.

For this case both of those options seem like a big extra layer of indirection and I don't see any risk of the mock affecting unintended code. Thoughts?

I think that the approach you have taken by mocking these internal functions which take care of the data download is actually the right way to test the emphasis of overwrite. Honestly when I write unit tests the motive is to make them run as isolated as possible and mocking functions that deal with a third party data download is one such step towards isolation.

The approach you took for checking if the overwrite option is actually working seems clean. File modification time is definitely a good way to go about it.

With that said I would have done something like using mockery to mock the download function to return different data (some random mock text) for subsequent calls. Then we check if the file text was indeed updated when overwrite was set to true on the function call.