dmorse / pscfpp

Polymer Self-Consistent Field Theory (C++/CUDA version)
https://pscf-home.cems.umn.edu
GNU General Public License v3.0
28 stars 20 forks source link

Grand canonical ensemble and unit testing. #44

Closed rpcollanton closed 2 years ago

rpcollanton commented 2 years ago
dmorse commented 2 years ago

Ryan - In the tests, please change

if (error) { UTIL_THROW(...) }

to

TEST_ASSERT(!error)

Rationale: Failing a TEST_ASSERT throws an exception that is caught by the unit test framework, stopping operation of that test and causing it to be recorded as failed, but doesn't cause the program to exit or crash. Throwing an uncaught exception crashes the unit test program. Marking the test as failed and continuing seems more appropriate. I'll accept the pull request after you make another small commit to the same branch with this change.

I don't think you need to do anything other than commit the change and tell me you've done so. I believe the pull request points to the current state of the originating branch, and so is automatically updated if that branch updates. Let's try this out in any case to make sure we both understand how to make and respond to requested changes in a pull request.

rpcollanton commented 2 years ago

Dave -- in SystemTest.h, I used TEST_THROW instead of UTIL_THROW. This halts execution and records a failure in the same way as TEST_ASSERT but allows a custom message. I felt it was appropriate since a message "!error" is somewhat nondescript. With this in mine, if you'd like me to change it still, I can do that!

rpcollanton commented 2 years ago

Also, I have messed around with PRs a bit and can confirm that it points to the current branch state so that if I make a new commit, it will be added to the PR.