GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/376
Test cleanup Following up after the migration of the test suite to JUnit Jupiter, some test code could be cleaned up to make it clearer and easier to maintain. This PR contains several patches around that area: General fixes: - In order to test an expected exception, `assertThrows` was used instead of a cumbersome `if`-`fail`-`catch` construct. - In order to fail a test on an unexpected exception, the exception is thrown outside the method, instead of the cumbersome construct of catching it and calling `fail` - Redundant `throws` clauses were removed - `assertEquals`, `assertNotEquals`, `assertSame`, `assertNotSame`, `assertNull` and `assertNotNull` were used instead of reimplementing them with conditions in `if` statements or `assertTrue`s. Specific improvements: - `ConverstionTest#assertBinaryEquals` was removed in favor of the built-in `Assertions#assertArraysEquals(boolean[], boolean[])`. - `LocaleUtilsTest#parseAllLocales` was rewritten as a parameterized test instead of implementing it manually with a loop. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang test-cleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/376.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #376 ---- commit c9ee985fce7d14d673d9b6bace824560eb4d40fc Author: Allon Mureinik <mureinik@...> Date: 2018-10-12T15:12:34Z Remove ConversionTest#assertBinaryEquals JUnit Jupiter's Assertions has an assertArraysEuqals(boolean[], boolean[]) method, so there's no longer a need for the assertBinaryEquals method. commit cf44c603b105f154b6b57604fe9abd589b7dbd2b Author: Allon Mureinik <mureinik@...> Date: 2018-10-12T16:14:01Z Make LocaleUtilsTest#parseAllLocales parameterized This patch converts testParseAllLocales to a @ParameterizedTest instead of iterating over the locales inside the method's body. This changes allows using standard asserts for each case individually instead of having to count failures and print the problematic locales to System.out. commit 15daf92088ad6d8868cd157ca9666721a59ce705 Author: Allon Mureinik <mureinik@...> Date: 2018-10-13T14:16:54Z Remove double stop() test in StopWatchTest StopWatchTest#testBadStates has the same block of code testing StopWatch#stop copy-pasted. This patch cleans it up by removing one of those blocks. commit 8507e5c81a8d3fedc655c02c93d4cf9dd4418ff6 Author: Allon Mureinik <mureinik@...> Date: 2018-10-13T14:08:48Z Clean up testing of exceptions Now that the entire project is ported to JUnit Jupiter, there are more elegant ways to test for exceptions, which this patch applies throughtout the code base. If throwing an exception is supposed to fail a test, just throwing it outside of the method cleans up the code and makes it more elegant, instead of catching it and calling Assertions#fail. If an exception is supposed to be thrown, calling Assertions#assertThrows is a more elegant option than calling Assertions#fail in the try block and then catching and ignoring the expected exception. Note that assertThrows uses a lambda block, so the variables inside it should be final or effectively final. Reusing variables is a common practice in the tests, so where needed new final variables were introduced, or the variables used were inlined. commit 3c6141d401233176d2e424640bffe0369592349e Author: Allon Mureinik <mureinik@...> Date: 2018-10-13T16:38:01Z Use assertTrue/assertFalse instead of reimplementing them Use assertTrue and assertFalse instead of reimplemeting their logic by having an if statement conditionalling call fail. commit 8ee1a558b821f28313b8b538b5d4b0de1b0e7044 Author: Allon Mureinik <mureinik@...> Date: 2018-10-13T16:49:52Z Clean up redundant throws clauses commit 591bebc111bca4f0681560b70fcc3d20cda40577 Author: Allon Mureinik <mureinik@...> Date: 2018-10-13T17:05:40Z Clean up assertions Use built-in assertion methods provides by org.junit.jupiter.api.Assertions instead of reimplementing the same logic with assertTrue and assertFalse. Note that JUnit Jupiter 5.3.1 does not support deltas of 0 for assertEquals(double, double, double) and assertEquals(float, float, float), so these usages of assertTrue were left untouched, and will be addressed when JUnit Jupiter 5.4, that addresses this isssue, will be available (see https://github.com/junit-team/junit5/pull/1613 for details). ---- ---