commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mureinik <...@git.apache.org>
Subject [GitHub] commons-lang pull request #376: Test cleanup
Date Sat, 13 Oct 2018 18:01:42 GMT
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).

----


---

Mime
View raw message