-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unit tests for pilot3utils function #13
Conversation
…e adding tests to get full test coverage of the functions.
Looking good!! Can you do devils::test-coverage() ? @bms - Once the nest_rowlabels( ) and efficacy_models( ) unit tests are committed, test-covr will be nearly 100% - the example.r is just a stub used if someone wishes to run test_file( ) for programs along a specific path. thx. @laxamanaj, @bms63 - any reason the lint.yml is setup this way in pilot3-utilities? lmk thx. |
I just grabbed lint checker from admiral. It can be re-configured if needed or simpler r-lib one can be used |
Hey Robert - sorry...styler and lintr are not aligned at the moment. We have been experiencing this in admiral - pharmaverse/admiral#2309 @bms63, @laxamanaj - successful through lintr - let's add the unit test for nest_rowlabels( ) and efficacy_models( ) then all set! |
@laxamanaj, @bms63 - a testthat example for nest_rowlabels( ) in Tplyr table is a challenge to find - possibly non-existent. |
We could either do snapshot testing or just call it a day!! https://testthat.r-lib.org/articles/snapshotting.html Thanks @bms63, expect_snapshot( ) and expect_snapshot_value( ) seem appropriate because it supports an accept workflow. Calling it a day. Thx, Ben. @bms63, @laxamanaj, @SHAESEN2 - the PASS snapshots can then be pushed to the repository as a point of truth for each of the functions then .gitignore _snaps/ so Pilot3 unit test snapshots are captured during the regulatory review as a one off. Maybe update the snapshots in _snaps when there is a new semantic version for the pilot3utils package and the battery of unit tests run again. thx. @bms63 , @laxamanaj, @SHAESEN2 - updated the DESCRIPTION file with the testthat(3e) setting. thx. @bms63 , @laxamanaj, @SHAESEN2, @kaz462 - all of Pilot3 Team and R Consortium Team and FDA Review Team will be happy to know the pilot3utils unit testing snapshots for the efficacy models function - the Pilot3 primary output in unit tests matches the (...wait for it.....) Pilot1 primary output. Hats off to Pilot3 Team and R Consortium Team and FDA Review Team. Just about done on this issue, we have about 100 unit tests and sample snapshots stored in _snaps directory. thx. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we need to rename the files to make it traceable eg in this file it seems you are testing adam format functions, rather than just functions
@SHAESEN2 - yes, the filenames are autogenerated and can be changed to make traceable to a requirements traceability matrix (RTM) for pilot3utils.
The admiral package has excellent test coverage already for the functions used in pilot3. The focus in this issue has been to unit test the functions defined in the pilot3utils package source. thx.
The unit test file names can be whatever pilot3 team wishes. With test_dir( ) every source file in the
specified directory gets tested with a "dot" output representing a function 'PASS' (see below). The _snaps
directory can be used for snapshotting test results for comparison. The testthat package creates test files in the
pilot3utils package using '.R' not '.r' extension - easy enough to change extension to match the regulatory specification. The traceability aspect to match the unit test to a specific pilot3 requirement is an excellent
point. One limitation (of using snapshots) is that snapshots are not aware if an output is correct - so an expect strategy comparing unit test output against 'correct' (point of truth) is necessary. thx.
The snapshots appear in the _snaps directory when the expect_snapshot unit test is run. Even a slight difference in expected outcome and previous test snapshot results in an error getting issued. An example using the pilot3utils nest_rowlabels function appears below using expect_snapshot and other testthat expect suggests.
It is pretty cool but the traceability matrix is a great idea then Pilot3 can link requirement to unit test PASS and store the snapshot in the repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those four lines are not testing the efficacy models?
@SHAESEN2 - yes, preparing to commit the unit test for the efficacy_models function. thx.
The four lines provide a unit test to confirm pilot3utils testthat(3e) is used - 3e is required for using snapshots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file we can include the character formats from before.
@SHAESEN2 - are you referring to the character formats here [earlier Issues 78, 87, and 91]. We will have to unit test those from the pilot3-adam repository.
Lmk, I could set it up testthat( ) in the pilot3-adam repository and unit test functions in the source where the 'xporter' and 'na' formatters are used from before. thx.
…plyr helper function nest_rowlabels() in pilot3utils.
…plyr helper function nest_rowlabels() in pilot3utils.
…ormat, helper, adam_functions in pilot3utils.
… the format, helper, adam_functions in pilot3utils.
…fficacy models in pilot3utils.
…fficacy models in pilot3utils.
…fficacy models in pilot3utils.
@laxamanaj, @bms63, @SHAESEN2, @kaz462 - we did get a successful snapshot of the efficacy models unit test. The data needed to test the function will be put into _snaps tomorrow so the GH R CMD Check doesn't fail when running the unit test in the action step to validate the PR. [The character formats @SHAESEN2 refers to are in the pilot3-adam repository - the testthat( ) setup will need to be put in over there to cover the character formats from before (adsl.r, etc.).] We're just about completed on the pilot3utils function, ~100% coverage. thx. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OMG @robertdevine way above the call of duty!! A Pilot Legend
…fficacy models in pilot3utils.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to call it!! LGTM!!
Using testthat( ) for automated testing of pilot3utils function. (CTRL+ SHIFT+L followed by CTRL+SHIFT+T)
@laxamanaj, @bms63 - still tidying testthat( ) for nest_rowlabels( ) and efficacy models( ) functions.
Modified all the R program names to *.r to meet regulatory specification regarding file name extensions for programs.