Concept question about JUnit Failures

Discussion in 'Java' started by Rhino, May 18, 2010.

  1. Rhino

    Rhino Guest

    I'm having a bit of a conceptual struggle right now with the right way to
    handle exceptions in JUnit tests. Since I always explain concepts better in
    terms of examples, let me describe a scenario to you which should
    illustrate the situation clearly.

    I have a method called getRGBColor() whose input parameter is a six letter
    String that is supposed to contain exactly six hex digits. Those hex digits
    are the String representation of an RGB Color. Therefore, "FFFFFF"
    represents White.

    I have no trouble writing a JUnit test that passes in "FFFFFF" as an input
    value and then verifies that the Color returned by the method is White i.e.
    Color(255,255,255) so I can test that aspect of the code without trouble.

    However, the method throws IllegalArgumentException if the input String is
    not precisely 6 characters long or if any of the characters are not hex
    digits.

    I can easily invoke the method with a value that will cause the exception
    to be thrown and I can catch it with a standard try/catch block. I can also
    put code in my catch block to make sure that JUnit reports the error from
    the Exception in the method being tested. I end up with this test which
    correctly reports the IllegalArgumentException within the JUnit window:

    try {
    Color actualRGBColor = colorConversionUtils.getRGBColor("FFFFFFA");
    Color expectedRGBColor = new Color(255, 255, 255);
    assertTrue("Actual color, " + actualRGBColor + ", does not equal expected
    color, " + expectedRGBColor, actualRGBColor.equals(expectedRGBColor));

    }
    catch (IllegalArgumentException ia_excp) {
    assertionFailedError = new AssertionFailedError(ia_excp.getMessage());
    assertionFailedError.initCause(ia_excp);
    throw assertionFailedError;
    }


    So far, so good. Now, here's where my problem comes in. When I execute this
    test, the method in which it is situated inevitably results in a black X
    decoration on the test result in the JUnit window of Eclipse. (I'm using
    JUnit3 for now.) I know that's reasonable given that it is correctly
    reporting an exception thrown by a method that is under test.

    However, I had the distinct impression in a conversion some months ago,
    that a properly written set of JUnit tests should always give me a green
    checkmark decoration on every test. And that makes a lot of sense to me
    too. Ideally, given that unit tests may be executed hundreds of times and
    each test class may have dozens of test methods in it, the last thing
    anyone wants to do is have to inspect each test method individually and
    know exactly which ones should have had green checkmarks, black X's and red
    X's. It's simply far easier to make sure that all of the tests had green
    checkmarks and only dive into them if some test _doesn't_ have a green
    checkmark.

    I don't see how I can reconcile these two ideas. If I test for thrown
    exceptions, I will inevitably get some black X's. (Unless there is some way
    to negate the exception test so that it only gives the black X if the
    exception FAILS to be thrown???) But if I get black X's, how am I to know
    quickly which of the test methods SHOULD be getting black X's and which
    ones should be getting green checkmarks? I don't much like the idea of
    trying to document all that and I certainly can't memorize it all....


    --
    Rhino
    Rhino, May 18, 2010
    #1
    1. Advertising

  2. Rhino

    Daniel Pitts Guest

    On 5/18/2010 1:21 PM, Rhino wrote:
    > I'm having a bit of a conceptual struggle right now with the right way to
    > handle exceptions in JUnit tests. Since I always explain concepts better in
    > terms of examples, let me describe a scenario to you which should
    > illustrate the situation clearly.
    >
    > I have a method called getRGBColor() whose input parameter is a six letter
    > String that is supposed to contain exactly six hex digits. Those hex digits
    > are the String representation of an RGB Color. Therefore, "FFFFFF"
    > represents White.
    >
    > I have no trouble writing a JUnit test that passes in "FFFFFF" as an input
    > value and then verifies that the Color returned by the method is White i.e.
    > Color(255,255,255) so I can test that aspect of the code without trouble.
    >
    > However, the method throws IllegalArgumentException if the input String is
    > not precisely 6 characters long or if any of the characters are not hex
    > digits.
    >
    > I can easily invoke the method with a value that will cause the exception
    > to be thrown and I can catch it with a standard try/catch block. I can also
    > put code in my catch block to make sure that JUnit reports the error from
    > the Exception in the method being tested. I end up with this test which
    > correctly reports the IllegalArgumentException within the JUnit window:
    >
    > try {
    > Color actualRGBColor = colorConversionUtils.getRGBColor("FFFFFFA");
    > Color expectedRGBColor = new Color(255, 255, 255);
    > assertTrue("Actual color, " + actualRGBColor + ", does not equal expected
    > color, " + expectedRGBColor, actualRGBColor.equals(expectedRGBColor));
    >
    > }
    > catch (IllegalArgumentException ia_excp) {
    > assertionFailedError = new AssertionFailedError(ia_excp.getMessage());
    > assertionFailedError.initCause(ia_excp);
    > throw assertionFailedError;
    > }
    >
    >
    > So far, so good. Now, here's where my problem comes in. When I execute this
    > test, the method in which it is situated inevitably results in a black X
    > decoration on the test result in the JUnit window of Eclipse. (I'm using
    > JUnit3 for now.) I know that's reasonable given that it is correctly
    > reporting an exception thrown by a method that is under test.
    >
    > However, I had the distinct impression in a conversion some months ago,
    > that a properly written set of JUnit tests should always give me a green
    > checkmark decoration on every test. And that makes a lot of sense to me
    > too. Ideally, given that unit tests may be executed hundreds of times and
    > each test class may have dozens of test methods in it, the last thing
    > anyone wants to do is have to inspect each test method individually and
    > know exactly which ones should have had green checkmarks, black X's and red
    > X's. It's simply far easier to make sure that all of the tests had green
    > checkmarks and only dive into them if some test _doesn't_ have a green
    > checkmark.
    >
    > I don't see how I can reconcile these two ideas. If I test for thrown
    > exceptions, I will inevitably get some black X's. (Unless there is some way
    > to negate the exception test so that it only gives the black X if the
    > exception FAILS to be thrown???) But if I get black X's, how am I to know
    > quickly which of the test methods SHOULD be getting black X's and which
    > ones should be getting green checkmarks? I don't much like the idea of
    > trying to document all that and I certainly can't memorize it all....
    >
    >


    public void testParseException() {
    try {
    myObj.getRGB("Bad value!");
    fail("Bad value did not cause exception!");
    } catch(IllegalArgumentException e) {
    // Success!
    }
    }

    public void testGoodParse() {
    assertEqual(white, myObj.getRGB("FFFFFF"));
    }


    --
    Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
    Daniel Pitts, May 18, 2010
    #2
    1. Advertising

  3. Rhino

    Tom Anderson Guest

    On Tue, 18 May 2010, Rhino wrote:

    > I can easily invoke the method with a value that will cause the
    > exception to be thrown and I can catch it with a standard try/catch
    > block. I can also put code in my catch block to make sure that JUnit
    > reports the error from the Exception in the method being tested. I end
    > up with this test which correctly reports the IllegalArgumentException
    > within the JUnit window:
    >
    > try {
    > Color actualRGBColor = colorConversionUtils.getRGBColor("FFFFFFA");
    > Color expectedRGBColor = new Color(255, 255, 255);
    > assertTrue("Actual color, " + actualRGBColor + ", does not equal expected
    > color, " + expectedRGBColor, actualRGBColor.equals(expectedRGBColor));
    >
    > }
    > catch (IllegalArgumentException ia_excp) {
    > assertionFailedError = new AssertionFailedError(ia_excp.getMessage());
    > assertionFailedError.initCause(ia_excp);
    > throw assertionFailedError;
    > }
    >
    > So far, so good.


    No, not good, this is completely wrong.

    You want this:

    @Test(expected=IllegalArgumentException.class)
    public void getRGBColorThrowsExceptionOnBadInput() {
    colorConversionUtils.getRGBColor("FFFFFFA");
    }

    > However, I had the distinct impression in a conversion some months ago,
    > that a properly written set of JUnit tests should always give me a green
    > checkmark decoration on every test.


    Correct.

    > If I test for thrown exceptions, I will inevitably get some black X's.
    > (Unless there is some way to negate the exception test so that it only
    > gives the black X if the exception FAILS to be thrown???)


    Got it in one.

    tom

    --
    I KNOW WAHT IM TALKING ABOUT SO LISTAN UP AND LISTEN GOOD BECUASE ITS
    TIEM TO DROP SOME SCIENTISTS ON YUO!!! -- Jeff K
    Tom Anderson, May 18, 2010
    #3
  4. Rhino

    Rhino Guest

    >
    > public void testParseException() {
    > try {
    > myObj.getRGB("Bad value!");
    > fail("Bad value did not cause exception!");
    > } catch(IllegalArgumentException e) {
    > // Success!
    > }
    > }
    >
    > public void testGoodParse() {
    > assertEqual(white, myObj.getRGB("FFFFFF"));
    > }
    >
    >


    Thank you VERY much for the speedy AND accurate answer! It never occurred
    to me to try that! It solves my problem very simply and elegantly. :)

    --
    Rhino
    Rhino, May 18, 2010
    #4
  5. Rhino

    Lew Guest

    On 05/18/2010 04:21 PM, Rhino wrote:
    > I'm having a bit of a conceptual struggle right now with the right way to
    > handle exceptions in JUnit tests. Since I always explain concepts better in
    > terms of examples, let me describe a scenario to you which should
    > illustrate the situation clearly.
    >
    > I have a method called getRGBColor() whose input parameter is a six letter
    > String that is supposed to contain exactly six hex digits. Those hex digits
    > are the String representation of an RGB Color. Therefore, "FFFFFF"
    > represents White.
    >
    > I have no trouble writing a JUnit test that passes in "FFFFFF" as an input
    > value and then verifies that the Color returned by the method is White i.e.
    > Color(255,255,255) so I can test that aspect of the code without trouble.
    >
    > However, the method throws IllegalArgumentException if the input String is
    > not precisely 6 characters long or if any of the characters are not hex
    > digits.
    >
    > I can easily invoke the method with a value that will cause the exception
    > to be thrown and I can catch it with a standard try/catch block. I can also
    > put code in my catch block to make sure that JUnit reports the error from
    > the Exception in the method being tested. I end up with this test which
    > correctly reports the IllegalArgumentException within the JUnit window:
    >
    > try {
    > Color actualRGBColor = colorConversionUtils.getRGBColor("FFFFFFA");
    > Color expectedRGBColor = new Color(255, 255, 255);
    > assertTrue("Actual color, " + actualRGBColor + ", does not equal expected
    > color, " + expectedRGBColor, actualRGBColor.equals(expectedRGBColor));
    >
    > }


    Your closing brace is supposed to line up at the same level as the 'try' command.

    > catch (IllegalArgumentException ia_excp) {


    The Java naming conventions decry the use of underscores in names of
    non-constant variables. Use camel case.

    You've seen this suggestion before, no?

    > assertionFailedError = new AssertionFailedError(ia_excp.getMessage());
    > assertionFailedError.initCause(ia_excp);
    > throw assertionFailedError;


    Why the heck are you throwing an error for the desired result?

    > }

    ....
    > I don't see how I can reconcile these two ideas. If I test for thrown
    > exceptions, I will inevitably get some black X's. (Unless there is some way
    > to negate the exception test so that it only gives the black X if the
    > exception FAILS to be thrown???) But if I get black X's, how am I to know
    > quickly which of the test methods SHOULD be getting black X's and which
    > ones should be getting green checkmarks? I don't much like the idea of
    > trying to document all that and I certainly can't memorize it all....


    If the test is supposed to throw an exception, then don't throw an
    AssertionFailedError in the 'catch' block. If the test is not supposed to
    fall through to the comparison with the expected color, then you are not
    expecting the color you assert to be expected.

    You will not "get a green check" if you lie about the expected results. Tell
    the truth instead; it works better.

    try
    {
    Color actualRGBColor = colorConversionUtils.getRGBColor("FFFFFFA");
    fail( "should have thrown IllegalArgumentException" );
    }
    catch ( IllegalArgumentException exc )
    {
    assertNotNull( "Null exception!?", exc );
    }

    --
    Lew
    Lew, May 18, 2010
    #5
  6. Rhino

    Tom Anderson Guest

    On Tue, 18 May 2010, Rhino wrote:

    >> public void testParseException() {
    >> try {
    >> myObj.getRGB("Bad value!");
    >> fail("Bad value did not cause exception!");
    >> } catch(IllegalArgumentException e) {
    >> // Success!
    >> }
    >> }
    >>
    >> public void testGoodParse() {
    >> assertEqual(white, myObj.getRGB("FFFFFF"));
    >> }

    >
    > Thank you VERY much for the speedy AND accurate answer! It never
    > occurred to me to try that! It solves my problem very simply and
    > elegantly. :)


    Not as elegantly as my solution!

    tom

    --
    The square-jawed homunculi of Tommy Hilfiger ads make every day an
    existential holocaust. -- Scary Go Round
    Tom Anderson, May 19, 2010
    #6
  7. Rhino

    Lew Guest

    Rhino wrote:

    and failed to acknowledge Daniel Pitts, who said:
    >>> public void testParseException() {
    >>> try {
    >>> myObj.getRGB("Bad value!");
    >>> fail("Bad value did not cause exception!");
    >>> } catch(IllegalArgumentException e) {
    >>> // Success!
    >>> }
    >>> }
    >>>
    >>> public void testGoodParse() {
    >>> assertEqual(white, myObj.getRGB("FFFFFF"));
    >>> }


    Rhino:
    >> Thank you VERY much for the speedy AND accurate answer! It never
    >> occurred to me to try that! It solves my problem very simply and
    >> elegantly. :)


    Tom Anderson wrote:
    > Not as elegantly as my solution!


    Indeed. As someone who is just now getting used to JUnit 4 and who loves,
    nay, adores annotations, I found tom's response exceedingly enlightening.

    --
    Lew
    Lew, May 19, 2010
    #7
  8. Rhino

    Daniel Pitts Guest

    On 5/19/2010 5:35 AM, Tom Anderson wrote:
    > On Tue, 18 May 2010, Rhino wrote:
    >
    >>> public void testParseException() {
    >>> try {
    >>> myObj.getRGB("Bad value!");
    >>> fail("Bad value did not cause exception!");
    >>> } catch(IllegalArgumentException e) {
    >>> // Success!
    >>> }
    >>> }
    >>>
    >>> public void testGoodParse() {
    >>> assertEqual(white, myObj.getRGB("FFFFFF"));
    >>> }

    >>
    >> Thank you VERY much for the speedy AND accurate answer! It never
    >> occurred to me to try that! It solves my problem very simply and
    >> elegantly. :)

    >
    > Not as elegantly as my solution!

    True, I'm not used to using Annotations.

    Although, the pattern I used can have additional benefits. It could be
    that you want to test the state of the object after the exception:

    public void testFoo() {
    try {
    foo.doSomething("broken");
    fail();
    } catch (BrokenException be) {
    // Success
    }
    assertThat(foo, hasGoodState());
    }

    --
    Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
    Daniel Pitts, May 19, 2010
    #8
  9. On 19-05-2010 14:47, Patricia Shanahan wrote:
    > Lew wrote:
    >> Rhino wrote:
    >> and failed to acknowledge Daniel Pitts, who said:
    >>>>> public void testParseException() {
    >>>>> try {
    >>>>> myObj.getRGB("Bad value!");
    >>>>> fail("Bad value did not cause exception!");
    >>>>> } catch(IllegalArgumentException e) {
    >>>>> // Success!
    >>>>> }
    >>>>> }
    >>>>>
    >>>>> public void testGoodParse() {
    >>>>> assertEqual(white, myObj.getRGB("FFFFFF"));
    >>>>> }

    >>
    >> Rhino:
    >>>> Thank you VERY much for the speedy AND accurate answer! It never
    >>>> occurred to me to try that! It solves my problem very simply and
    >>>> elegantly. :)

    >>
    >> Tom Anderson wrote:
    >>> Not as elegantly as my solution!

    >>
    >> Indeed. As someone who is just now getting used to JUnit 4 and who
    >> loves, nay, adores annotations, I found tom's response exceedingly
    >> enlightening.

    >
    > I have a slight problem with using the annotation approach alone. It can
    > only check for a throw of appropriate type. It cannot check other
    > features of the thrown object, such as its message or a wrapped
    > exception. As far as I can tell, a catch block is the only way to fully
    > test if the interface promises anything beyond a throw type.


    Checking the message can be necessary, but it is an indication that
    some programmer were too reluctant to subclass.

    Arne
    Arne Vajhøj, May 20, 2010
    #9
  10. Rhino

    Tom Anderson Guest

    On Wed, 19 May 2010, Patricia Shanahan wrote:

    > Lew wrote:
    >> Rhino wrote:
    >>
    >> and failed to acknowledge Daniel Pitts, who said:
    >>>>> public void testParseException() {
    >>>>> try {
    >>>>> myObj.getRGB("Bad value!");
    >>>>> fail("Bad value did not cause exception!");
    >>>>> } catch(IllegalArgumentException e) {
    >>>>> // Success!
    >>>>> }
    >>>>> }
    >>>>>
    >>>>> public void testGoodParse() {
    >>>>> assertEqual(white, myObj.getRGB("FFFFFF"));
    >>>>> }

    >>
    >> Rhino:
    >>>> Thank you VERY much for the speedy AND accurate answer! It never
    >>>> occurred to me to try that! It solves my problem very simply and
    >>>> elegantly. :)

    >>
    >> Tom Anderson wrote:
    >>> Not as elegantly as my solution!

    >>
    >> Indeed. As someone who is just now getting used to JUnit 4 and who loves,
    >> nay, adores annotations, I found tom's response exceedingly enlightening.

    >
    > I have a slight problem with using the annotation approach alone. It can
    > only check for a throw of appropriate type. It cannot check other
    > features of the thrown object, such as its message or a wrapped
    > exception. As far as I can tell, a catch block is the only way to fully
    > test if the interface promises anything beyond a throw type.


    Very true. Daniel's point about not being able to make assertions about
    the throwing object after the throw is also a good one. I suppose i'd
    retreat slightly and say the expected exception method is appropriate when
    you just want to check that the right exception is thrown, because it's
    easy, it makes the intent clear, and it gives you a nice error message,
    but if you want to do anything more, it's no use.

    It also runs the risk of silently letting bugs pass in situations like
    this:

    @Test(expected=IndexOutOfBoundsException.class)
    public void testGetThrowsExceptionForBadIndex() {
    List<String> l = new MyList();
    l.add("hello");
    l.get(1);
    }

    This is written to check that get correctly throws an exception - but if
    add incorrectly throws an exception, it will still pass.

    So now i'm thinking about retreating even further.

    tom

    --
    OK, mostly because of Tom, but not only because of his bloody irritating
    character and songs.
    Tom Anderson, May 20, 2010
    #10
  11. Rhino

    Arne Vajhøj Guest

    On 18-05-2010 17:03, Tom Anderson wrote:
    > On Tue, 18 May 2010, Rhino wrote:
    >> I can easily invoke the method with a value that will cause the
    >> exception to be thrown and I can catch it with a standard try/catch
    >> block. I can also put code in my catch block to make sure that JUnit
    >> reports the error from the Exception in the method being tested. I end
    >> up with this test which correctly reports the IllegalArgumentException
    >> within the JUnit window:
    >>
    >> try {
    >> Color actualRGBColor = colorConversionUtils.getRGBColor("FFFFFFA");
    >> Color expectedRGBColor = new Color(255, 255, 255);
    >> assertTrue("Actual color, " + actualRGBColor + ", does not equal expected
    >> color, " + expectedRGBColor, actualRGBColor.equals(expectedRGBColor));
    >>
    >> }
    >> catch (IllegalArgumentException ia_excp) {
    >> assertionFailedError = new AssertionFailedError(ia_excp.getMessage());
    >> assertionFailedError.initCause(ia_excp);
    >> throw assertionFailedError;
    >> }
    >>
    >> So far, so good.

    >
    > No, not good, this is completely wrong.
    >
    > You want this:
    >
    > @Test(expected=IllegalArgumentException.class)
    > public void getRGBColorThrowsExceptionOnBadInput() {
    > colorConversionUtils.getRGBColor("FFFFFFA");
    > }


    If he can upgrade from JUnit 3 to 4.

    Otherwise the fail method as show by Daniel Pitt must be the
    solution.

    JUnit 4 is relative old now, so I can not see any reasons not
    to upgrade (at least not for a project where the work to convert
    is not a problem in itself).

    Arne
    Arne Vajhøj, May 26, 2010
    #11
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Kevin Spencer

    Re: So many failures early on

    Kevin Spencer, Aug 13, 2003, in forum: ASP .Net
    Replies:
    4
    Views:
    324
    Kevin Spencer
    Aug 27, 2003
  2. Guest
    Replies:
    0
    Views:
    406
    Guest
    Jun 19, 2004
  3. tommy
    Replies:
    0
    Views:
    350
    tommy
    Jul 19, 2004
  4. jlambrecht
    Replies:
    0
    Views:
    551
    jlambrecht
    Dec 22, 2004
  5. MikeB
    Replies:
    4
    Views:
    339
    Owen Jacobson
    Oct 26, 2004
Loading...

Share This Page