9

I am trying to write a test case for the method decrypt here.

    private static Codec codec;

    static {
        try {
            codec = new Codec(encryptionType, encryptionKey, false, true, false);
        } catch (CodecException e) {
            throw new RuntimeException("Codec initialisation failed", e);
        }
    }


    public static String decrypt(final String toDecrypt) throws CodecException {
        String decrypted = codec.decryptFromBase64(toDecrypt);
        if (decrypted.endsWith(":")) {
            decrypted = decrypted.substring(0, decrypted.length() - 1);
        }
        return decrypted;
    }

Test Case:

    @Mock
    private Codec codec;
    @Test
    public void test_decrypt_Success() throws CodecException {
        when(codec.decryptFromBase64(TestConstants.toDecrypt)).thenReturn(TestConstants.decrypted);
        assertEquals(DocumentUtils.decrypt(TestConstants.toDecrypt), TestConstants.decrypted);
    }

Since this is a static method, I can't inject an instance of the class in the test suite and mock its codec. The above code throws an error from the codec library at assert as expected.

What is your approach to testing static methods like this? Or should I not be writing tests for this at all?

tanvi
  • 927
  • 5
  • 17
  • 32
  • Possible duplicate of [Mock/Stub super constructor invocation in Java for unit testing](https://stackoverflow.com/questions/19255332/mock-stub-super-constructor-invocation-in-java-for-unit-testing) – talex Dec 10 '18 at 10:39
  • Use PowerMock to mock static methods. But you are not even calling the decrypt(final String toDecrypt) method in your Junit. Without that, the statement when(codec.decryptFromBase64(TestConstants.toDecrypt)).thenReturn(TestConstants.decrypted); has no meaning. – A_C Dec 10 '18 at 10:43
  • @AnkurChrungoo my bad. Updated the q. I meant to call decrpyt in assert. – tanvi Dec 10 '18 at 10:46
  • @talex, I am not trying to mock a constructor or a super constructor. I want to mock a static variable initialised in static block in a static method. – tanvi Dec 10 '18 at 10:48

3 Answers3

5

In Java, static methods are not designed to set dependencies.
So switching the dependency into a mock is really not natural.
You could provide a static setter for the field such as :

private static Codec codec;
public static void setCodec(Codec codec){
   this.codec = codec;
}

And you could set a mock with setCodec(...) but ugh...

But forget, just do things well : refactor the code to remove all static and introduce a constructor that sets the codec.

private Codec codec;
public MyClassUnderTest(Codec codec){
   this.codec codec;
}

IOC could help here to make the class under test a singleton and ease the dependency injections.
If not possible in your case, Java 5 enumeration could help you for at least the singleton concern.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
  • I was not using the static eariler. But a peer pointed out that this is a util class and as such I should be initialising the codec only once. How do we write mocks for util static classes? – tanvi Dec 10 '18 at 10:52
  • 1
    @tanvi I suspected the same! You have some choices like writing proper object oriented code and move this code into a separate class which does this specific job and non-statically...... or write the static setter and adapt the code accordingly...or use PowerMockito to mock static methods:) Refer https://blog.codecentric.de/en/2011/11/testing-and-mocking-of-static-methods-in-java/ for PowerMockito example. – A_C Dec 10 '18 at 10:55
  • @AnkurChrungoo how would you suggest I adapt the code according to the static setter? – tanvi Dec 10 '18 at 10:59
  • 2
    I agree with a single initialization but I don't about static usage. Ok in Java static methods are easy to write and simplify client usage for util methods. But it also hs some limitations : this hardcodes dependencies and so make it no naturally switchable. I generally write static methods only for methods that never need to mock any things. About your question, the setter is a way. PowerMock is another one. But I would avoid both in your case as you are the api/component developer. – davidxxx Dec 10 '18 at 11:02
  • @tanvi, like David has suggested. – A_C Dec 10 '18 at 11:02
  • 1
    @davidxxx, yes, I would rather suggest separating this code out into a singleton class since it has a codec dependency. This shall make it easier to test it/mock it. – A_C Dec 10 '18 at 11:05
  • @tanvi, create a separate singleton class (follow SOLID principles) and use it for this functionality. I think that will serve the purpose and also help you test it more easily. – A_C Dec 10 '18 at 11:09
  • @AnkurChrungoo I do not want to make my class under test a singleton as it is a utils final class. I do not own the Codec class. – tanvi Dec 10 '18 at 11:19
  • @Ankur Chrungoo I agree completely. This makes things clean. – davidxxx Dec 10 '18 at 11:22
  • @tanvi The fact that the dependency is not in your base code will never prevent you from defining a singleton that uses that. – davidxxx Dec 10 '18 at 11:25
  • @davidxxx This is a final utils class for which I do not want to create a singleton. All the methods in the class are static. – tanvi Dec 10 '18 at 11:26
  • @tanvi, if you do not want to modify, then you should use the other options that david and me mentioned. (static setter/ PowerMockito). These are kind of hacks which are needed for mocking and testing static methods/classes. – A_C Dec 10 '18 at 11:28
  • 1
    If you don't want to change your mind, stay on my initial comment : powermock or the static setter. – davidxxx Dec 10 '18 at 11:29
  • Thanks @davidxxx. I will go with a setter and mock the getter. – tanvi Dec 10 '18 at 11:30
2

There are many different and shortcut ways of achieving the same (as pointed out in comments and other answers), but not all of them are good in the long run especially.

I would suggest creating a Singleton class which implements the Decrypt functionality. So, you won't have to create multiple instances, and don't really need to have static method for decrypting as well, and you can inject your codec once and more easily (I assume you don't have multiple types of codec as per your comments. But, if you do, then the functionality shall be adapted accordingly).

For more reference: Why use a singleton instead of static methods?

For reference on why static should be used carefully:- Why are static variables considered evil?

A_C
  • 905
  • 6
  • 18
  • Agree concerning the main idea but in Java I prefer IOC to old singleton way with the static `getInstance()` method. – davidxxx Dec 10 '18 at 11:24
  • @davidxxx, yes, but many times that doesn't suit us, although that is the most ideal way in the long run. – A_C Dec 10 '18 at 11:25
1

In my experience in those cases I just prepare all instances in a @Before method:

private Codec codec;

@Before
public void setup() throws CodecException {
  codec = new Codec(encryptionType, encryptionKey, false, true, false);
}
Juan Ramos
  • 557
  • 4
  • 14