DataQualityDashboard icon indicating copy to clipboard operation
DataQualityDashboard copied to clipboard

Move from travis to github actions

Open clairblacketer opened this issue 4 years ago • 7 comments
trafficstars

We need continuous integration testing using github actions

clairblacketer avatar Jan 21 '21 17:01 clairblacketer

Hello @clairblacketer and @fdefalco ! I'm part of the Kheiron developers cohort and for my first project @ablack3 suggested I could try taking on this issue. Do you all still need help with this? If so please let me know and I'd be happy to jump in :)

I spent the last week familiarizing myself with GH Actions and unit testing in R, and (assuming you'd like my help) my next step will be to fork the DQD repo and start figuring out an implementation plan. I'll let you all know how I'm thinking of approaching this before diving into the work to make sure we're all aligned.

katy-sadowski avatar Jun 21 '22 23:06 katy-sadowski

@katy-sadowski YES PLEASE! I have no idea how to implement GitHub actions and unit tests and it is one of the main items keeping us from considering dqd a hades package. Thank you!!!

clairblacketer avatar Jun 22 '22 10:06 clairblacketer

Great! I will get started on this and keep you posted on my progress.

katy-sadowski avatar Jun 22 '22 18:06 katy-sadowski

Here is my PR: https://github.com/OHDSI/DataQualityDashboard/pull/338

I look forward to your feedback!

katy-sadowski avatar Jul 01 '22 22:07 katy-sadowski

I'm just trying to run a single test.

  results <- executeDqChecks(connectionDetails = Eunomia::getEunomiaConnectionDetails(),
                             cdmDatabaseSchema = "main",
                             resultsDatabaseSchema = "temp",
                             cdmSourceName = "Eunomia",
                             checkLevels = "TABLE",
                             writeToTable = F)

This does work when I run it interactively. However when I run this using devtools::test() (all other tests commented out) I get the error

Error in `$<-.data.frame`(`*tmp*`, "call_text", value = c("DataQualityDashboard::executeDqChecks(...)",  : 
  replacement has 15 rows, data has 7
Calls: <Anonymous> ... trace_format_branch -> trace_as_tree -> $<- -> $<-.data.frame
Execution halted

Any ideas why the test passes when I run it interactively but not when I run devtools::test()?

ablack3 avatar Jul 05 '22 20:07 ablack3

Here is the only test I'm running. It works when I run it interactively but not when I run devtools::test().

library(testthat)

test_that("Execute DQ checks on Synthea/Eunomia", {
  results <- executeDqChecks(connectionDetails = Eunomia::getEunomiaConnectionDetails(),
                             cdmDatabaseSchema = "main",
                             resultsDatabaseSchema = "temp",
                             cdmSourceName = "Eunomia",
                             checkNames = "measurePersonCompleteness",
                             writeToTable = F,
                             numThreads = 1,
                             verboseMode = TRUE
  )
  expect_true(nrow(results$CheckResults) > 1)
})

devtools::test() error message: Error in $<-.data.frame(*tmp*, "call_text", value = c("DataQualityDashboard::executeDqChecks(...)", : replacement has 15 rows, data has 7 Calls: <Anonymous> ... trace_format_branch -> trace_as_tree -> $<- -> $<-.data.frame Execution halted

ablack3 avatar Jul 05 '22 20:07 ablack3

I found the culprit: loadRenderTranslateSql. This function does not work well with devtools because it calls system.file with a parameterized package name. I'd recommend not using it and instead explicitly loading the sql files. I removed it and the problem went away.

I've made a few changes to @katy-sadowski's PR to get tests working. I commented out the tests on the remote databases for now as well as the tests that seem to take a long time to run.

I think we should come up with a strategy around testing DQD. Ideally unit tests would run in a couple minutes at most. The quicker we can make them the more often developers can run them. I'm wondering if anyone has ideas for meaningfully testing the functionality without running all checks on several database platforms. Or maybe we can have two sets of tests - short tests for developers to run while working and long running comprehensive tests to run before a release.

@katy-sadowski Will you pull my changes into your branch? https://github.com/ablack3/DataQualityDashboard/tree/issue195 I based this off of your work so there should not be any conflicts.

ablack3 avatar Jul 05 '22 23:07 ablack3