dpc10ster / RJafroc

Artificial Intelligence: Evaluating AI, optimizing AI
19 stars 8 forks source link

Add simple tests using the testthat package #5

Closed pwep closed 5 years ago

pwep commented 5 years ago
codecov[bot] commented 5 years ago

Codecov Report

Merging #5 into development will increase coverage by 21.11%. The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development      #5       +/-   ##
===============================================
+ Coverage         0.08%   21.2%   +21.11%     
===============================================
  Files               56      56               
  Lines             7325    7325               
===============================================
+ Hits                 6    1553     +1547     
+ Misses            7319    5772     -1547
Impacted Files Coverage Δ
R/SsPowerTable.R 1.23% <0%> (+1.23%) :arrow_up:
R/PlotEmpiricalOperatingCharacteristics.R 2.64% <0%> (+2.64%) :arrow_up:
src/MyFOM.cpp 9.88% <0%> (+9.09%) :arrow_up:
R/BinningRelated.R 9.3% <0%> (+9.3%) :arrow_up:
R/DfReadDataFile.R 32.81% <0%> (+32.81%) :arrow_up:
R/addArguments.R 33.33% <0%> (+33.33%) :arrow_up:
R/gpfMyFOM.R 43.42% <0%> (+43.42%) :arrow_up:
R/ChisqrGoodnessOfFit.R 55.31% <0%> (+55.31%) :arrow_up:
R/SsPowerGivenJK.R 70.29% <0%> (+70.29%) :arrow_up:
R/StSignificanceTesting.R 70.62% <0%> (+70.62%) :arrow_up:
... and 11 more

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 0cc50c3...c78dcd8. Read the comment docs.

dpc10ster commented 5 years ago
  • testthat is already suggested in DESCRIPTION
  • a tests directory and testthat.R file has been created
  • aim to have tests for all major R functions, grouped by R file name
  • some tests check a function correctly issues an error
  • use devtools::test() to run the tests within R

Should we use the hash method and forget about the detailed comparison of all output?

pwep commented 5 years ago

I can see you have added some helpful tests, that exceed anything I offer here in this pull request!

I have created an issue in your repository (#6) relating to expect_known_output() and a pull request (#7) that I believe fixes the issue.

You might want to close this pull request without merging, as my offered tests are simplistic, and you have (re)added extensive tests to this development branch. There will be other potential issues that need to be solved, based on the messages coming from your tests.

pwep commented 5 years ago

In fact I shall close the pull request now.

dpc10ster commented 5 years ago

Thanks. I added the tests ~ Dec 2018 leading up to my last successful CRAN upload. I removed them recently, as some of them started failing for reasons that I have yet to resolve. I will continue working on making all tests pass, perhaps using the hash as a quick fix. I intend to create a small reproducible example showing the failure of testthat (specifically function expect_known_output) that I will pose to the R developer community. Dev

On Jun 21, 2019, at 20:21, Peter Phillips notifications@github.com wrote:

In fact I shall close the pull request now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/5?email_source=notifications&email_token=AH4NJRDGS72NK4MQ7ESBEWDP3VWAPA5CNFSM4HZNHUZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYJ3XMQ#issuecomment-504609714, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRF2WSWK2YGMQC4M32LP3VWAPANCNFSM4HZNHUZA.

dpc10ster commented 5 years ago

On Jun 21, 2019, at 20:20, Peter Phillips notifications@github.com wrote:

I can see you have added some helpful tests, that exceed anything I offer here in this pull request!

I have created an issue in your repository (#6 https://github.com/dpc10ster/RJafroc/issues/6) relating to expect_known_output() and a pull request (#7 https://github.com/dpc10ster/RJafroc/pull/7) that I believe fixes the issue.

I missed this. I am going to look into it right now.

You might want to close this pull request without merging, as my offered tests are simplistic, and you have (re)added extensive tests to this development branch. There will be other potential issues that need to be solved, based on the messages coming from your tests.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dpc10ster/RJafroc/pull/5?email_source=notifications&email_token=AH4NJRBGOVXL4K7BSZODRZTP3VV5BA5CNFSM4HZNHUZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYJ3WWI#issuecomment-504609625, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NJRETSJIXUSY345K6HLDP3VV5BANCNFSM4HZNHUZA.