Write unit tests for the functions

[bms63]

No description provided.

[robertdevine]

QC_FINDINGS_CONFIRMATION_SUMMARY_REPORT_USING_expect_dfs_equals.txt

@bms63 and @laxamanaj, appropriate to develop the unit tests for the functions using the admiraldev reference here. PR should get issued sometime next week for review. Chipping away at the remaining issues so everything is in good order for when Regulatory Reviewers issue their Pilot3 Review and Report. thx.

The testthat( ) automation may be helpful for Pilot3 as well. Also, testing the {admiral} functions (interested to unit test the LOCF derivation flagged by the regulatory review). The {admiraldev} function expect_dfs_equal is helpful.

Using the {admiraldev} function to test FDA Review Observation here, per the cited discrepancies in 'primary output' from Pilot1 and Pilot3.

Per Pilot3 QC Findings, "The R-generated ADADAS matches original adadas from CDISC pilot data, except for the records where PARAMCD=ACTOT, DTYPE=LOCF. This is an issue from the CDISC ADADAS." Testing with {admiraldev} unit test functions to re-confirm. (@laxamanaj, @kaz462, @bms63)

Unit testing does confirm the QC Findings conclusion concerning the discrepancies cited by FDA Reviewers. The confirmation is demonstrated using the expect_dfs_equal( ) {admiraldev} unit test. (Output report attached - verbose)

To generate a full report, please issue the command below from console or within expect statement using testthat:
>try(expect_dfs_equal(adadas, qc_adadas, keys = c("USUBJID", "PARAMCD", "AVISIT", "ADT")))

With testthat( ), Pilot3 Team provides systematic testing so if anyone wants to modify functional codes then the package automatically catches breaking changes - quality checks, downstream dependency safety checks, data integrity, as discussed by the WG. We'll drop in ~100 unit tests covering each function across the R source files in pilot3utils. The {admiraldev} unit tests should be helpful. Using tibble[::tribble] samples from the Subject-Level Analysis Dataset to unit test Tplyr-helper functions (e.g. nest_rowlabels, bind_rows) and helper functions (e.g. pad_row, num_fmt). thx.

[bms63]

oh that would be pretty cool!!

[laxamanaj]

Thanks, @robertdevine !

[robertdevine]

@laxamanaj, @bms63, @kaz462 - we pretty much have the pilot3 testthat( ) unit tests completed including a confirmation test using 'expect_dfs_equal( )' to support the QC Findings conclusion and address the FDA Reviewers observations regarding primary output discrepancies between pilot1 and pilot3. We have good test coverage over the pilot3utilities functions. Will issue PR later this week. thx.

@kaz462 - unit testing does replicate the QC Findings at each step in the ADADAS section of the Pilot3 document - the 'expect_dfs_equal( )' function generates a nearly identical summary report (attached - verbose) to diffdf( ) when comparing 'adadas' and 'qc_adadas'. Thank you again.

[robertdevine]

@laxamanaj, @bms63 - the unit testing in Pilot3 also appears to be a community contribution for a sample of efficacy functionality from the CDISC Pilot Replication PHUSE Test Data Factory ADaM data, using the R programming language repository. Many thanks to CDISC Pilot replication for providing the functions. Pretty cool to see the community collaboration with ADaM data. thx.

[bms63]

Awesome @robertdevine looking forward to reviewing. It would be nice for our package to be a little snazzier when we re-submit. Felt a little janky when submitted in the Fall!!

[bms63]

Hey Robert - just checking on timeline for this? I don't see a separate branch for this - is this being done locally? We want to re-submit soon, let me know if you need some help on finish this out. Many thanks again!!

[robertdevine]

Hi @bms63, adding tests to include coverage with latest unmerged changes from PR Issue #146 yesterday, should be able to push later today or latest on Monday using local branch pilot3utils-unit-tests per the screenshots above. Test coverage approaches 100%. thx.

@bms63, @laxamanaj - as far as timeline for this Issue #2 unit tests for pilot3utils, the issue is tracked on the Pilot3 [project board] https://github.com/orgs/RConsortium/projects/7/views/1) as 'In Progress' since January 26, 2024. The API for the repo does not appear enabled to return query parameters for timeline events. The Pilot3 issue timeline events are not parameterized with the GH REST API feature to track timeline events for tracking issue timelines, just the project board without the event triggers. If you run a query the response is empty. (below) thx.

@bms63, @laxamanaj - actually collaborator access to the submissions-pilot3-utilities repository is needed for me to push the automated unit test codes. Collaborator access is granted through Collaborators tab under Settings. thx.

The unit tests are run in automation from developer's console at any time by issuing the command sequence:
CTRL+SHIFT+L followed by CTRL+SHIFT+T [i.e. Essentially, LOAD ALL+TEST]

We also ran test_check( ) [0 Errors 0 Warnings 1 Note] which runs correctly for the package in under one minute.

Having the systematic testing built into the workflow does assist the developer experience while building out pilot3utils package - i.e. protects against downstream dependency breaking changes, etc.. Overall, pretty cool. thx.

*The additional test coverage to include derive_locf_records( ) function is due to discrepancies noted by FDA Reviewers here (please see Item 2 on the document) and addressed in QC Findings under the ADADAS section. thx.

**Nice to have would be to address the NOTE with additional systematic checks for the suggested but not required here.

[robertdevine]

@laxamanaj, @bms63, @kaz462 - for the {admiral} functions [like derive_locf_records} the unit testing with the testthat( )
uses a pattern of namespace::function( ) notation when testing the ADaM data sets etc. within pilot3utils package
automated testing. thx.

[bms63]

Thanks Robert! Let me know when the PR is ready and we can merge it in ASAP.

Wondering...Do we need the derive_var_locf function anymore in pilot3utils? I was under the impression that admiral had incorporated this change - @kaz462 can you confirm.

[robertdevine]

@bms63 - actually collaborator access to the submissions-pilot3-utilities repository is needed for me to push the automated unit test codes. Collaborator access is granted through Collaborators tab under Settings. thx.

[bms63]

@bms63 - actually collaborator access to the submissions-pilot3-utilities repository is needed for me to push the automated unit test codes. Collaborator access is granted through Collaborators tab under Settings. thx.

Added you in

Thanks @bms63, added a branch for this issue and will create PR using the issue branch today.

[kaz462]

Wondering...Do we need the derive_var_locf function anymore in pilot3utils? I was under the impression that admiral had incorporated this change - @kaz462 can you confirm.

@bms63 you are right, this was incorporated in admiral, so we don't need derive_var_locf in pilot3utils

@bms63, @kaz462 - The {admiral} test coverage is excellent - 680+ unit tests using testthat( ) - here. thx.

[bms63]

@robertdevine can you remove the pilot3utils function and test in this branch/PR for us?

@bms63 - will do. thx.

[robertdevine]

Please see discussion here. Then we'll wrap this up and have test coverage over R/user_utils.R as well since they were used earlier in the pilot3 but not explicitly in pilot3utils. package thx.