adw96 / DivNet

diversity estimation under ecological networks
83 stars 18 forks source link

Increase Unit testing #105

Closed paulinetrinh closed 2 years ago

paulinetrinh commented 2 years ago

I increased code coverage for S3 functions by covering more of the H0 options in testBetaDiversity. I updated the NAMESPACE, started fixing the phylodivnet.R file and created a test-phylodivnet.R thinking that I would increase unit testing onphylodivnet()... However, after going through phylodivnet() I don't think that function is completed so it can't be covered by testing. phylodivnet() takes up about 12% (96 lines) of DivNet's code so we should consider either completing the function or something.

codecov-commenter commented 2 years ago

Codecov Report

Merging #105 (c833534) into master (f6cf209) will increase coverage by 10.80%. The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #105       +/-   ##
===========================================
+ Coverage   65.95%   76.75%   +10.80%     
===========================================
  Files          13       13               
  Lines         796      796               
===========================================
+ Hits          525      611       +86     
+ Misses        271      185       -86     
Impacted Files Coverage Δ
R/phylodivnet.R 0.00% <0.00%> (ø)
R/s3functions.R 96.26% <0.00%> (+40.18%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f6cf209...c833534. Read the comment docs.

paulinetrinh commented 2 years ago

Codecov has two different commit statuses: project and patch. The project status reviews the entire project's coverage to maintain minimum requirements. The patch status assesses if each line adjusted in a pull request is covered by tests. This helps your team increase code coverage with each pull request.

^^For our reference. I was trying to fix phylodivnet.R and add testing coverage for that function, only to find out that the function does not work. I committed my changes to phylodivnet.R and added in the test-phylodivnet.R testing script without any real tests and am unable to hit the codecov/patch target of 66%, which is why it failed.

adw96 commented 2 years ago

@paulinetrinh If I recall correctly, we abandoned phylodivnet after deciding that it was computationally infeasible. I think the best thing to do would be to remove it from the code base -- if someone ever needs it they can search the history looking for it. What do you think of this strategy?

paulinetrinh commented 2 years ago

@adw96 That sounds good to me! I was going to bring up the options to you and see what you wanted to do but I can remove it from the code base as long as you're ok with that!

paulinetrinh commented 2 years ago

@adw96 I should mention additionally that there is another script divnet_alpha_estimates.R that has no code coverage. I'm not sure if it's used anywhere (doesn't seem like it?) or if it's completed. Should we leave that in the code base?

adw96 commented 2 years ago

Hmmmm... It looks like it might be useful. Could you please see if there are any implications of removing it by creating a branch with it removed and seeing if that branch passes checks?

(I expect that if we haven't documented it anywhere then others are probably not using it either)

paulinetrinh commented 2 years ago

Sounds good! It's actually not in the NAMESPACE so people are definitely not using it.

adw96 commented 2 years ago

Hmmmm... it may be internal though... Does breakaway use it? (perhaps with DivNet:::divnet_alpha_estimates()?)

paulinetrinh commented 2 years ago

I'm not seeing the function make_alpha_estimates() written in divnet_alpha_estimates.R being called anywhere in DivNet or breakaway's repositories, but I'll make a separate branch to test its removal.

Before that I will: