Closed jdhoffa closed 3 years ago
I understand that testing the entire dataset will be very slow, so maybe an alternative is to test against a random sample of rows of the dataset?
I don't think expect_snapshot(list(a, b, c))
is slower than expect_snapshot(a); expect_snapshot(b); expect_snaphsot(c)
. At least my intent in snapshotting the list was not to save computation time but to avoid duplication. But my approach is likely an off-label use of snapshots -- so I'm very happy to implement what you are asking for.
test that all the datasets are actually identical either in CI or some other check that isn't run very often.
FYI you can skip slow tests locally with the environment variable PACTA_SKIP_SLOW_TESTS=TRUE
(default). I set it in a project-specific .Renviron file (see usethis::edit_r_environ("project")
).
Testing this on CI is a bit complex because the github action needs access to the docker image and to the private repo pacta-data/. I think it's doable but will take a lil while to develop.
I understand that testing the entire dataset will be very slow ...
Yeah, testing the snapshot of each dataset in full seems to be slow (~190s), apparently because printing and comparing large datasets takes a while. But the test was quite slow (~50s) because running pacta takes a long while too. As the workflow is not snappy anyway, for now I prefer to wait a bit more and have the most comprehensive regression test we can. This will be less important when we have tests of other kind, and then we may test just a piece of each dataset.
I think it is important that, before refactoring, we expand this snapshot test into individual snapshot tests that against each dataset individually.
I understand that testing the entire dataset will be very slow, so maybe an alternative is to test against a random sample of rows of the dataset?
And then we can have a slowly more complete test that all the datasets are actually identical either in CI or some other check that isn't run very often.
Originally posted by @jdhoffa in https://github.com/2DegreesInvesting/pactaCore/pull/49#r701106915