PROPOSAL: Migrate AD template tests from CI workflow to testthat

[ddsjoberg]

Background Information

Rather than having tests of the templates tucked into a CI workflow, it would be great to have it as a part of the general testthat frame work. This is pretty simple do to with snapshot testing. This would ensure that the derived ADSL has not changed after updates are made. Quick example below:

expect_snapshot({
  fs::path_package("admiral", "templates", "ad_adsl.R") |> 
    source(local = TRUE)
  adsl |> print(n = Inf, width = Inf)
})

There are a number of workflows that occur with each PR, and simplifying would be great.

Definition of Done

No response

[manciniedoardo]

We are currently discussing execution of templates here as well in relation to {admiraldata}.

[bms63]

@cicdguy and @galachad what do you think of using snapshots?

[cicdguy]

@cicdguy and @galachad what do you think of using snapshots?

No strong opinions on using snapshots. They definitely have their advantaged and disadvantages, but it's one of the standard forms of testing. I'll let Adam express his opinions as he's more well-versed with snapshot-based testing.

[bms63]

I think this would help simplify the Pull Request process as well. As we wouldn't have to Approve and wait for the CI Template check to pass anymore.

[bundfussr]

We used a workflow for running the templates because we did not want to run them each time something is pushed (running the templates takes some time).

I think the crucial question is: How do we ensure that the snapshot is correct?

Another question: What is returned by expect_snapshot() if the result is not as expected?

[bms63]

Perhaps @ddsjoberg can put together a small demo for one of the templates and we can discuss more at Wednesday meeting?

[ddsjoberg]

Sure, let's review snapshots Wednesday

[bms63]

I remember this got tabled for now. Going to move to Discussions

[manciniedoardo]

Closing due to lack of activity.