insightsengineering / tlg-catalog

A catalog of Tables, Listings and Graphs (TLGs) created with NEST R packages
https://insightsengineering.github.io/tlg-catalog/
Other
20 stars 8 forks source link

adding tests draft #54

Closed Melkiades closed 1 year ago

Melkiades commented 1 year ago

fixes #46

@shajoezhu @clarkliming @pawelru I did it for half of a complex template (4 variants). It is clear by the mismatches in the pagination results that there is already a divergence happening. Of course, here it is top left text and sorting but it could become more problematic. The commented code in the text file shows the tests from scda.test.

All tests can be run by devtools::test() or something like devtools::test_file("tests/testthat/test_table_aet04.R"). An idea could be to add this to the downstream tests. Do you think something like this is possible to do @cicdguy ?

cicdguy commented 1 year ago

An idea could be to add this to the downstream tests. Do you think something like this is possible to do @cicdguy ?

Yes, definitely on the agenda to execute tests in chunks for these files. We used to do this, we haven't enabled it here. It's on the backlog to enable it, however.

pawelru commented 1 year ago

All tests can be run by devtools::test()

This is great. If test() is working then how about R CMD CHECK? If it can be run and it includes the test then maybe we can even run our current check action? Can you try to add it to the workflow yaml file just for try? If not then we would have to have a new test-only action.

Melkiades commented 1 year ago

All tests can be run by devtools::test()

This is great. If test() is working then how about R CMD CHECK? If it can be run and it includes the test then maybe we can even run our current check action? Can you try to add it to the workflow yaml file just for try? If not then we would have to have a new test-only action.

I think we need a test-only action here that also builds the book beforehand (to generate the data)

output from devtools::check() (R CMD check does not work from terminal -> not a pkg)

E  checking extension type
   Extensions with Type ‘Book’ cannot be checked.
pawelru commented 1 year ago

Thanks for checking this. Then we would have to request for another action that needs to be run only after successful render as you pointed out. This all looks good to me. Waiting for others to express opinions

Melkiades commented 1 year ago

Thanks for checking this. Then we would have to request for another action that needs to be run only after successful render as you pointed out. This all looks good to me. Waiting for others to express opinions

This is the best option imo. Still, I was thinking about it, and at the end of each quarto file there could be hidden testing, i.e. a devtools::test_file(paste0(test_dir, "/test_table_aet04.R")). So every time it is rendered, it is also tested.

pawelru commented 1 year ago

After a while, I have to admit that I am less and less convinced to do this. Mostly because moving the whole data-intensive testing into TLGC would make it responsible for both rendering and testing which will break single responsibility paradigm. Generally speaking it's advised to follow divide&conquer strategy and split functionals into self-contained, encapsulated pieces. This will be very much against it as there are very different goals for TLGC and tests. And I am telling that with a heavy heart as I know all the drawbacks of the current setup :( I hate just complaining and not giving any alternatives but I really don't have any right now. Will have to think more about it. Happy to hear what other think about it.

Nevertheless, there is something nice here which is testability of TLGC outputs. It's a quite nice feature that is missing currently. I think we should definitely go this way. But this does not address the main problem here.

pawelru commented 1 year ago

Hey All. I took some of the ideas and tried out an approach of package in subdir that makes use of symlinked book. Please have a look here: https://github.com/insightsengineering/tlg-catalog/pull/62

The goal was to allow R CMD CHECK and R CMD TEST which then could be used in reverde dependency check in tern and other. This is a gist.

Melkiades commented 1 year ago

Superseded by https://github.com/insightsengineering/tlg-catalog/pull/62