Refactor testWrapping.cpp
#3544
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@pepbos, @adamkewley, since you've been recently adding new wrapping tests, you might have some thoughts/opinions here. |
|
@nickbianco I'd name the file deprecated for future reference or restoration. Most of these unused tests I'm not familiar with but a refactoring of unused tests should probably need no more than one reviewer. If actual tests are preserved then feel free to merge |
aymanhab
approved these changes
Sep 12, 2023
There was a problem hiding this comment.
Reviewed 4 of 6 files at r1, all commit messages.
Reviewable status: 4 of 6 files reviewed, all discussions resolved
|
Seems reasonable - assuming the test suite wasn't testing anything too useful 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relevant to #3387, #3389
Brief summary of changes
This PR refactors
testWrapping.cpp, which was previously a bit of a mess, to now use theCatch2framework. The new structure should make it easier to expand the test suite for wrapping code without having to create a bunch of new files. To that end, I've moved the tests fromtestMuscleLengthRegression.cppintotestWrapping.cpp.There was a lot of code that wasn't being tested at all. I've temporarily moved this code to a new file,
sandboxWrapping.cpp, until we decide what to do with it.Looking for feedback on...
What should we do with the unused code moved to
sandboxWrapping.cpp? Change the filename (i.e., "deprecated") so the code is saved? Revive the unused tests? Delete it altogether?CHANGELOG.md (choose one)
This change is