Convert Simulation tests to catch2#3897
Convert Simulation tests to catch2#3897AllisonJohn wants to merge 14 commits intoopensim-org:mainfrom
Conversation
nickbianco
left a comment
There was a problem hiding this comment.
A couple minor suggestions, but looks good overall!
Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AllisonJohn)
OpenSim/Simulation/Test/testInverseKinematicsSolver.cpp line 243 at r1 (raw file):
noiseLevel*double(noise(gen)), SimTK::ZAxis ); cout << "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << endl;
Remove?
OpenSim/Simulation/Test/testMomentArms.cpp line 348 at r1 (raw file):
SimTK::Vec2(-2*SimTK::Pi/3, SimTK::Pi/18), 0.0, "VASINT of BothLegs with no mass: FAILED"); cout << "VASINT of BothLegs with no mass: PASSED\n" << endl;
It would be better to remove all these subtests into SECTIONs and remove the cout statements. When running the test suite, the sections will show as passed or failed.
Also, watch the 80 column limit.
AllisonJohn
left a comment
There was a problem hiding this comment.
Reviewable status: 10 of 12 files reviewed, 2 unresolved discussions (waiting on @nickbianco)
OpenSim/Simulation/Test/testInverseKinematicsSolver.cpp line 243 at r1 (raw file):
Previously, nickbianco (Nick Bianco) wrote…
Remove?
Done.
OpenSim/Simulation/Test/testMomentArms.cpp line 348 at r1 (raw file):
Previously, nickbianco (Nick Bianco) wrote…
It would be better to remove all these subtests into
SECTIONs and remove thecoutstatements. When running the test suite, the sections will show as passed or failed.Also, watch the 80 column limit.
Done.
|
I need to add LoadOpenSimLibrary("osimActuators"); back to some files |
nickbianco
left a comment
There was a problem hiding this comment.
A few more suggestions to clean things up.
Reviewed 2 of 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AllisonJohn)
OpenSim/Simulation/Test/testModelInterface.cpp line 38 at r3 (raw file):
TEST_CASE("testModelFinalizePropertiesAndConnections") { LoadOpenSimLibrary("osimActuators");
Replace with include?
OpenSim/Simulation/Test/testMomentArms.cpp line 347 at r3 (raw file):
testMomentArmDefinitionForModel("BothLegs22.osim", "r_knee_angle", "VASINT", SimTK::Vec2(-2*SimTK::Pi/3, SimTK::Pi/18), 0.0, "VASINT of BothLegs with no mass: FAILED");
Similar to removing the cout statements, we should remove the need for an error message from testMomentArmDefinitionForModel since the Catch framework will tell us if the subtest passed or failed.
OpenSim/Simulation/Test/testStatesTrajectory.cpp line 266 at r3 (raw file):
TEST_CASE("testPopulateTrajectoryAndStatesTrajectoryReporter") { LoadOpenSimLibrary("osimActuators");
If every test needs this library, we should just include it above rather than loading it for every test.
|
@AllisonJohn let me know if you wanted to finish up this PR or if you want me to take it over. |
Fixes issue #3555
Brief summary of changes
Converted remaining test files in the simulation folder to use catch test cases
Testing I've completed
tests still pass
Looking for feedback on...
CHANGELOG.md (choose one)
This change is