search

metrumresearchgroup / pkgr

R package installation and management - reimagined.
https://metrumresearchgroup.github.io/pkgr/docs
39 stars 4 forks source link

v0.5.0 Impact assessment #146

Closed dpastoor closed 4 years ago

dpastoor commented 5 years ago

current pkgr status

pkgr 0.5.0-beta has been used in production environments internally + externally without known issue for the last month or so. However, these environments generally have been simplistic (eg pull from a couple cran-like repos with minimal to no customizations present).

At this point, we must assess the completeness of our testing harness(es), especially with respect to edge cases or particular functionality not necessarily used day-to-day but is available.

The findings will be captured in additional comments below.

Assignments

david-lyder commented 5 years ago

Completeness of tests with respect to customization options

For convenience, here is the definition from the SDLC for the Impact assessment: The Impact Assessment considers all activities to be implemented in the User Story and assesses the product risk that implementation of this User Story will adversely impact the system/software.

We have pretty good "end-to-end" test coverage for the customization options, however there are some gaps WRT unit tests. For clarity, when I say "end-to-end" tests, I am referring to the tests in the "integration_ tests" folder.

The table below lists info for each customization option.

Customization feature Existing Integration (end-to-end) Test Unit test effort Risk Notes
Packages\Type mixed-source small low Existing function "pkgNexus.SetPackageType" very simple function, can be unit tested easily
Packages\Repo pkgr-multirepo small low Existing function "pkgNexus.SetPackageRepo" very simple function, can be unit tested easily
Packages\Env internal master small low There is an existing TODO to create a function. The function looks to be trivial and unit-testable
Packages\Suggests simple-suggests medium medium Medium effort to create unit test for existing (recursive) function appendToGraph
Packages\Repos simple-suggests pkgrlinkingTo small low Remove about 8 lines of code from planInstall and create unit test-able function
Repos\Type pkgrlinkingTo large medium Requires refactor of planInstall and creating test-able functions and unit tests. (planInstall customization code accesses both the cfg object and uses Viper to read the cfg tree)
dpastoor commented 5 years ago

@david-lyder fanastic work - I think it definitely makes sense to put some light unit tests around all these. can you please work with @Dreznel to break up the work as appropriate, especially around the planInstall refactor

david-lyder commented 5 years ago

Sure, I will create a branch and work with @Dreznel

Dreznel commented 5 years ago

Following a format similar to Dave:

Test Case Existing end-to-end tests Existing unit tests Risk Todo Note
Install from Local Repositories None None medium Create a local drat-repo in the integration_tests folder. Create test cases that use this DRAT repo. I believe there is already a cran_local folder that is supposed to server as this, I am not certain if it is set up correctly, though. We may be able to leverage that, and then revive some old test cases from the "needs work" folder.
Install from Remote Repositories Covered in multiple integration tests (simple, simple-suggests, the update None low Update integration tests to use Microsoft CRAN snapshots for simple-suggests, outdated-pkgs, outdated-pkgs-no-update, repo-order  
Install from Partial CRAN-like Repositories Covered in mixed-source, but mixed-source itself could use a bit of cleanup None low Document expected results for mixed-source and make sure that test in generally in a runnable state. Swap out mrgsolve for pmplots  
Install from Mix of Local and Remote Repositories Since we don't have tests for local repositories, we have nothing for a mix of local and remote. None medium Once we have a local drat repo we can use, create an integration test that installs both binary and source packages from both a local and a remote repo  

We don't really have any unit test coverage for these areas specifically. Since pulling a package from a remote or local repository is a higher-level test case, I think it makes sense to let these be covered by integration tests for now, at least until we've refactored more of the internals.

dpastoor commented 5 years ago

@Dreznel thanks - agree with you that these are not really something we need to concern ourselves with unit-testing wise. I think it is a good find that we are lacking in the testing of local repos. Ironically, we should definitely use that a bit in our tests as it will speed up and reduce the dependency on a (good) internet connection to keep tests running smoothly.

Actionably, we should add some local repo tests, we can just use incomplete cranlike repos, no need to do something too drastic.