Closed kyle-messier closed 7 months ago
@sigmafelix @shail-choksi @ericbair-sciome @sciome-bot I merged an updated PrestoGP_Model.R function. I've excluded 3 linters you can see on the linting yaml. Nonetheless, linting is and will continue to cause some substantial changes to code, though if done correctly will not impact the results. Perhaps that makes the unit and integration tests more important. Please see PrestoGP_Model.R for the changes. I suggest using the Code>Reformat-Code option of R-Studio that takes care of lots of linting.
Yeah, I am working on writing a thorough set of testthat tests to ensure that all code works correctly after we make changes. Are those supposed to be run by GitHub when we commit things? Because it looks like it isn't working now. In any event, I'm wondering if we should hold off on making any cosmetic lint changes until all of of our testing infrastructure is in place. That way we can make sure that nothing accidentally gets broken. By the way, if GitHub automatic testing isn't working, you can always run devtools::test() manually.
@ericbair-sciome The test-coverage and R-CMD-build tests are currently being automatically run on pull requests and commits to main. I think the idea of getting unit and build tests in place before linting is not a bad idea. What do you think @shail-choksi @sciome-bot ?
@ericbair-sciome In the meantime- PrestoGP_Model.R was linted and merged with main, so you could test that to make sure it still works as expected.
Am I looking at the wrong thing? The test-coverage page I was looking at was the following:
It looks like the testthat tests are not even being run because there are some configuration errors or something. Like I said, the workaround is to just run devtools::test() from within R, but if those could be run automatically in GitHub, that would be cool.
@ericbair-sciome exactly - the test-coverage yaml is setup to run in the GitHub action is all I meant. You are correct in that it is failing because other package related stuff.
Will help ensure compatibility with CRAN