Hi,

On 29.05.19 00:10, sebb wrote:
On Tue, 28 May 2019 at 21:44, Karl Heinz Marbaise <khmarba...@gmx.de> wrote:

Hi,

just a small comment of mine.

to get very good support for exceptions etc. I would suggest to use
assertj which will work in JUnit 4 and Jupiter (aka 5)...

What License does it use?

https://github.com/joel-costigliola/assertj-core/blob/master/LICENSE.md

Apache License
Version 2.0, January 2004
http://www.apache.org/licenses/


JUnit 5  has the following License:

Eclipse Public License - v 2.0

https://github.com/junit-team/junit5/blob/master/LICENSE.md


Furthermore I would not rely on a unit testing framework for assertions
which others can do better like assertj does.

Snippets from the Docs:

assertThatThrownBy(() -> { throw new Exception("boom!"); })
.isInstanceOf(Exception.class)
.hasMessageContaining("boom");

or:

assertThatExceptionOfType(IOException.class)
.isThrownBy(() -> { throw new IOException("boom!"); })
.withMessage("%s!", "boom")
.withMessageContaining("boom")
.withNoCause();

Real code example:

assertThatIllegalArgumentException()
    .isThrownBy(() -> Complex.parse(re + "," + im + ")"));
(This is the equivalent of using
@Test(expected=IllegalArgumentException.class))...
but you can also check the message very easy:

assertThatIllegalArgumentException()
    .isThrownBy(() -> Complex.parse(re + "," + im +
")")).withMessage("....");

Another code example:

current code:
Assert.assertTrue(Complex.ofCartesian(re, im).equals(Complex.parse(str)));

Should not be doing comparisons that way as the expected and actual
values are not shown...

Yes I know but this is code I found...(current state)

https://github.com/apache/commons-numbers/blob/master/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexTest.java#L889


And this it would looks like with AssertJ:

assertThat(Complex.parse(str)).isEqualTo(Complex.ofCartesian(re, im));

Note that the expected and actual values are the other way round from JUnit.
This could cause some confusion; care will need to be taken when converting.

Especially since there is likely to be existing code with expected and
actual the wrong way round.
I found (and fixed!) quite a lot of that in Commons code in the past;
I expect there is more.

In assertj there is only one way (also based on natural reading):

assertThat(actual).isEqualTo(expected);

Apart from that by using some IDE's the conversion can be done
automatically but of course the conversions have to be checked...


Kind regards
Karl Heinz Marbaise


Furthermore (current code):

          Assert.assertTrue(Double.isNaN(nanZero.getArgument()));
          Assert.assertTrue(Double.isNaN(zeroNaN.getArgument()));
          Assert.assertTrue(Double.isNaN(NAN.getArgument()));

Again, surely one would use a comparison with expected and actual?

via AssertJ:

          assertThat(nanZero.getArgument()).isNaN();
          assertThat(zeroNaN.getArgument()).isNaN();
          assertThat(NAN.getArgument()).isNaN();


Furthermore about  the subject for running JUnit 4 and JUnit Jupiter
(aka 5) in one build can be simply handled by just using the following
dependencies instead of JUnit 4:

    <dependencies>
      <dependency>
        <groupId>org.junit.jupiter</groupId>
        <artifactId>junit-jupiter-engine</artifactId>
        <version>5.4.2</version>
        <scope>test</scope>
      </dependency>
      <dependency>
        <groupId>org.junit.vintage</groupId>
        <artifactId>junit-vintage-engine</artifactId>
        <version>5.4.2</version>
        <scope>test</scope>
      </dependency>
    </dependencies>

That gives you the opportunity to write your Tests with JUnit 4 and also
with JUnit Jupiter aka JUnit 5. This would help to get a smooth
transition to JUnit 5.


I have created a branch
https://github.com/khmarbaise/commons-numbers/tree/JUNIT5-ASSERTJ where
you can take a look how such tests could look like and running together
JUnit 4 / JUnit Jupiter (aka JUnit 5)...also
some examples how using assertj looks like in particular for assertions
about Exceptions inlcuding the messages.

(Just as an example not meant to be a real pull request cause that is
not finished yet. This can be improved much more)...

Those tests show me the exception messages in CoseAngle need some
improvement (missing space).

* ComplexTest using assertj assertions
* CosAngleTest using junit 5 + assertj assertions..


Kind regards
Karl Heinz Marbaise

[1]:
https://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html#exception-assertion

On 22.05.19 18:34, Heinrich Bohne wrote:
Right now, commons-numbers is using JUnit 4.12, the last stable version
of JUnit 4. As far as I am aware, there is no explicit syntax in JUnit
4.12 for testing whether an exception is thrown apart from either using
the deprecated class ExpectedException or adding the "expected"
parameter to the Test annotation. The problem with the latter approach
is that it is impossible to ascertain where exactly in the annotated
method the exception is thrown – it could be thrown somewhere unexpected
and the test will still pass. Besides, when testing the same exception
trigger with multiple different inputs, it is impractical to create a
separate method for each test case, which would be necessary with both
aforementioned approaches.

This has led to the creation of constructs where the expected exception
is swallowed, which has been deemed undesirable
<https://issues.apache.org/jira/browse/NUMBERS-99?focusedCommentId=16843419&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16843419>.

Because of this, I propose to add JUnit 5 as a dependency in
commons-numbers. JUnit 5 has several "assertThrows" methods that would
solve the described dilemma.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to