File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Testing and the fly likes How to test unknown enum value? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Engineering » Testing
Bookmark "How to test unknown enum value?" Watch "How to test unknown enum value?" New topic
Author

How to test unknown enum value?

Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5534
    
  13

When trying to increase the test coverage on our project I ran into an interesting issue with enums. Assume the following code:



It's fairly easy to write a test case to verify the calculate method is producing the expected results for addition and subtraction. But if you want complete test coverage for this method you also need a test case for the IllegalArgumentException. How can you achieve such a test? (all testing frameworks are allowed)

I know the implementation of the calculate method is not really object oriented and I should refactor the above code and create a method to the enum to perform the logic. And because we are the owner of the enum, refactoring is exactly what I did in our project But if we were using some enum from a 3rd party (which we can't modify) and we are very keen on 100% test coverage, we have to create such a test.


SCJA, SCJP (1.4 | 5.0 | 6.0), SCJD
http://www.javaroe.be/
Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4991
    
    8

In the case of this example, I'd question the validity of the choice to throw an IllegalArgumentException with a message "operator [op] unknown". Since the op parameter is declared as an Operator, then the compiler guarantees it will be a valid value. The question is whether it's a value that's supported by the calculate() method. If anything, I would change the message to be "operator [op] is not supported". When you do this, the unit test to cover the exception getting thrown is pretty obvious and suddenly makes more sense vs when you have a message that says "operator unknown".


Junilu - [How to Ask Questions] [How to Answer Questions]
Tim Cooke
Bartender

Joined: Mar 28, 2008
Posts: 1173
    
  65

The line where you throw the IllegalArgumentException is non reachable code. Your enum has two values, your switch statement has cases for both values, both cases return from the method. There is no way to get past the switch statement.

I recommend you delete everything after the switch statement. If it's not there you don't need to test it.


Tim Driven Development
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5534
    
  13

Junilu Lacar wrote:If anything, I would change the message to be "operator [op] is not supported".

You are correct. The message of the exception is poorly chosen and this one is a much better one. But that doesn't solve the original problem
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5534
    
  13

Tim Cooke wrote:The line where you throw the IllegalArgumentException is non reachable code. Your enum has two values, your switch statement has cases for both values, both cases return from the method. There is no way to get past the switch statement.

No, it isn't. Without this line the code would not compile, because your method expects an int to be returned.
Tim Cooke
Bartender

Joined: Mar 28, 2008
Posts: 1173
    
  65

Well that seems a bit silly. I would consider that a bit of a compiler quirk that it doesn't recognise that you can never get there. In that case it is impossible to get test coverage for that bit of code.

You have two choices:
  • You can just accept it and move on.
  • You can decide that your implementation may not be the best design and take a different, more testable, approach.
  • Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    Roel De Nijs wrote:The message of the exception is poorly chosen and this one is a much better one. But that doesn't solve the original problem


    Pseudocode, assuming JUnit4:



    You might want to create and use another RuntimeException instead, like UnsupportedOperatorException for example, with a field to hold the operator that caused the exception. One thing to note: if you have no unsupported values, then the test will not be able to exercise the throwing of the exception anyway and you won't get 100% coverage. You'll only get 100% coverage if the exception will actually get thrown, which is what I think Tim's point is.
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    Tim Cooke wrote:In that case it is impossible to get test coverage for that bit of code.[/list]


    Not impossible. Just need to step outside the box a little bit
    Stuart A. Burkett
    Ranch Hand

    Joined: May 30, 2012
    Posts: 679
    Junilu Lacar wrote:One thing to note: if you have no unsupported values, then the test will not be able to exercise the throwing of the exception anyway and you won't get 100% coverage. You'll only get 100% coverage if the exception will actually get thrown, which is what I think Tim's point is.

    I think that was the point of the original question. The switch handles all the values in the enum so how can the exception throwing be tested as there are no values that won't be handled by the switch.
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    If that was the point of the original question, I'd have to agree with Tim in that it's a bit silly to expect to have a way to cover that bit of code with a test if there's no possibility of it ever being executed. Any valid test would have to use values that would actually cause the code under test to throw the exception. Otherwise, the test would just be there as a "safety net" in case a new enum value is added in which case you'd find that you suddenly have 100% coverage.
    Tim Cooke
    Bartender

    Joined: Mar 28, 2008
    Posts: 1173
        
      65

    Junilu Lacar wrote:Not impossible. Just need to step outside the box a little bit

    OK. I've stepped outside the box. I'm looking around but I'm not seeing anything. Now I've lost the box..... Can I get a clue?
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    Tim Cooke wrote:I'm looking around but I'm not seeing anything. Now I've lost the box..... Can I get a clue?

    So you don't think the pseudocode I gave would be a valid test when there are enum values that are not supported by the calculate method?
    Tim Cooke
    Bartender

    Joined: Mar 28, 2008
    Posts: 1173
        
      65

    Yes. But I don't see how you can get unsupported enum values into the calculate method without adding them to the production code.
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    Tim Cooke wrote:Yes. But I don't see how you can get unsupported enum values into the calculate method without adding them to the production code.


    I'm not sure I follow your line of thinking here. I'm assuming that the given code snippet is just an example similar to the actual code or an abbreviation of the actual code. Also assuming that the actual enumerated type has more values and that some values are not supported. Otherwise, there really is no basis for expecting to get 100% test coverage as I mentioned before.
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5534
        
      13

    Stuart A. Burkett wrote:I think that was the point of the original question. The switch handles all the values in the enum so how can the exception throwing be tested as there are no values that won't be handled by the switch.

    That was indeed the original question!

    Junilu Lacar wrote:I'd have to agree with Tim

    I also totally agree with Tim: using switch statement is in most cases not the best design, so it can be refactored to a more testable approach. And that's what i did in the current project. But some curiosity kicked in: what to do if I could not refactor this code, because e.g. the enum was from a 3rd party library and I can't change it? With mocking frameworks you can mock static methods and private methods, so I was just wondering if it was possible to mock some bogus enum constant and use that one to cover this line. If this test can be added, then the build will fail if a new enum constant was added (in a new version of this 3rd party library) but not covered in our method. So the "broken" method will not make it to production before it was fixed.
    Tim Cooke
    Bartender

    Joined: Mar 28, 2008
    Posts: 1173
        
      65

    Right ok. I wasn't making any assumptions and was taking the op's code as is with two enum values, both handled in the switch.

    I see right enough that if the prod enum contained unsupported values then testing would be trivial.
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5534
        
      13

    Junilu Lacar wrote:Also assuming that the actual enumerated type has more values and that some values are not supported.

    My mistake: I mislead you completely with the post about the exception message. I didn't mean "unsupported" as in "I have 5 enum values and this method handles 2, so I want an exception when invoked with one of the other 3", but "unsupported" as in "I have 5 enum values, all covered in this method. But I want to throw an exception if a 6th value is added to the enum and I forget to handle this new enum value in the method". That's why I used "unknown" in the original post. Sorry for that!
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    I think you should modify your approach. Instead of expecting the test for the exception being thrown to fail, you should write another test that explicitly checks for the number of values that your current code expects there to be:



    This test will fail when the number of enumerated values changes. At that point, you have to re-examine your design and existing tests and change the hard-coded value as necessary so that future changes can be flagged.
    Paul Clapham
    Bartender

    Joined: Oct 14, 2005
    Posts: 18882
        
        8

    I see there's a lot of people saying what I originally thought but never got around to posting: if you have code which can't be tested, then perhaps there's a design problem.
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    In this case, Paul, I don't think it's about design and untestable code. The goal is to have a way to detect that certain assumptions have been invalidated or need to be adjusted. If a new enumerated value is added, then there's a possibility that test/production code should be changed. The first test that I suggested would cover the exception being thrown for unsupported values. The "flagger" test would tell the developer that a review of the current design and code should be done.
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5534
        
      13

    Junilu Lacar wrote:Just need to step outside the box a little bit.

    That's what I did when I started experimenting with the testing and mocking frameworks I know And it really helped! I created a test case using the latest versions of JUnit, EasyMock and PowerMock which gives 100% test coverage for the Calculator class from the original post.

    Here is the code:
    Junilu Lacar
    Bartender

    Joined: Feb 26, 2001
    Posts: 4991
        
        8

    A few comments:
    1. I prefer to write tests that check for one thing -- your calculate test asserts three different things. I would break this up into three separate tests.
    2. I don't see how this test will flag the fact that there is a new enum value if the enumerated type changes. To verify this, add a new operator enum value in the real code and run the test again. I'm pretty sure it will still pass.
    3. I don't think the 100% coverage goal is an absolute need for the example you gave. It seems to me that it's more important to get a reminder that you need to check your assumptions and implementation again when a change to the enumerated type is detected. This is what the flagger test that I suggested would do.
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5534
        
      13

    Junilu Lacar wrote:I prefer to write tests that check for one thing -- your calculate test asserts three different things. I would break this up into three separate tests.

    If you want to verify the Operator class I don't see any option other than grouping all asserts in 1 test. I tried the AfterClass annotation, but that didn't work well.

    Junilu Lacar wrote:I don't see how this test will flag the fact that there is a new enum value if the enumerated type changes. To verify this, add a new operator enum value in the real code and run the test again. I'm pretty sure it will still pass.

    It will only pass if you expect the IllegalArgumentException to be thrown, otherwise your test will fail. Demonstrated in the test case at the bottom of this post.

    Junilu Lacar wrote:I don't think the 100% coverage goal is an absolute need for the example you gave. It seems to me that it's more important to get a reminder that you need to check your assumptions and implementation again when a change to the enumerated type is detected. This is what the flagger test that I suggested would do.

    It's indeed more important to get a reminder, how it's achieved is less important. But if you are using some code quality tool like Sonar, this test case gives you 100% test coverage and that's what we try to achieve for every project (and that's why I started wondering about this scenario in the 1st time)

    Added a multiply operator:


    Slightly updated the test case (to be completely independent on how much values the enum has + each assert has its own test). This test runs successfully.
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: How to test unknown enum value?