Add unit tests#553
Conversation
garydgregory
left a comment
There was a problem hiding this comment.
Hello @sean-g-rae
This PR needs work. You must make sure that when a change is made to a main class, a corresponding test class fails when the main changes are omitted.
For example, if I apply this PR locally and revert the changes to ColorGroup and run ColorGroupTest, the test is green. This obviously is bad and means that a regression is just waiting to happen.
TY!
|
@sean-g-rae ping 🔔 |
What: Roll back the implicit cast, isolating this PR to just adding unit tests Why: Change was not covered by unit tests, and didn't fit the rest of the PR scope
Thanks for the feedback! To keep the PR focused, I reverted the non-test change. |
|
Hello @sean-g-rae The build is broken From the description: Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself. |
A version of PR #553 that doesn't break the build.
|
Closing: I cleaned up a local diff of this PR that doesn't break the build. You are credited in git master. |
Thanks for your contribution to Apache Commons! Your help is appreciated!
Before you push a pull request, review this list:
mvn; that'smvnon the command line by itself.