[FIXED] Junit Test Case for private method

Issue

I have a below class. I am new in writing junit tests. I need to write the test case for this. How can I write test method for startSchemaMaintenance method in the test class as it is calling a private method with no args?

public class SchemaMaintenance {

    //Loading statuses overview
    // NOT_STARTED = 0
    // START_LOADING = 1
    // IN_PROGRESS = 2
    // COMPLETED = 3
    // LOADING_ERROR = 4
    private static volatile Integer loading_status = 0;

    public void startSchemaMaintenance() throws Exception {
        if (checkLoadingStatus() == 1) {
            doSchemaMaintenance();

            loading_status = 3;
        }
    }

    private void doSchemaMaintenance(){
        //Do something....
    }

    private int checkLoadingStatus() throws Exception {
        if (loading_status==0 ||loading_status == 2) {
            synchronized (loading_status) {
                if (loading_status==0) {
                    loading_status = 2;
                    return 1;
                }else if(loading_status == 2) {
                    while(loading_status == 2);

                    if((loading_status == 4)){

                        throw new Exception("status = " + 4);
                    }
                }else if(loading_status == 4) {

                    //log.error(generateErrorMessage());
                    throw new Exception("status = " + 4);
                }
            }
        }else if((loading_status == 4)){
            //log.error(generateErrorMessage());
            throw new Exception("status = " + 4 );
        }

        return loading_status;

    }
}

Solution

First of all a couple of things:

1. Why is loading_status static? Basically 2 instances of this class would use the same loading_status variable. loading_status should either not be static or checkLoadingStatus should be static as well.
2. loading_status is terribly named. If you want to keep it static, you should name it LOADING_STATUS. If it won't be static, let's name it loadingStatus. That's the Java convention.
3. You should create a proper enum type for loading_status instead of it being an integer.
4. while (loading_status == IN_PROGRESS); is just simply bad practice. At least what you can do is: while(loading_status == IN_PROGRESS) { Thread.sleep(100); }; However a timeout is a better practice. Sometehing like:

long timeoutMillis = 5000L; // Should come from a configuration or something
long endTimeMillis = System.currentTimeMillis() + timeoutMillis;
while (loadingStatus == IN_PROGRESS) {
    long remainingMillis = end - System.currentTimeMillis();
    if (remaining <= 0) {
        // Timeout while loading
        loadingStatus = LOADING_ERROR;
        break;
    }
    Thread.sleep(50);
}
// ...

And finally I guess something will update loadingStatus to completed or something. I guess you are using synchronized there as well. If the loading happends on 2 different threads then you will end-up in a dead-lock. If checkLoadingStatus enter the synchronized block first, so when the loading is completed the loading thread will never be able to enter the synchronized block, because the checkLoadingStatus is holidng the lock.

Last but not least to answer you question, if you want to keep your method private, you can invoke it through reflection, but that's again bad practice. Generally you should avoid unit testing private methods and unit test those methods which are invoking it. If however you definitely need to unit test a particular method, make it package-private instead of private, and then you can create a unit test in the same package where the class is which contains your method. And you can add a comment to the method saying it's only visible for unit testing. Example of your code structure in this case:

src
+-- main
+-- +-- com/app/MyClass.java
+-- test
    +-- com/app/MyClassTest.java // this test class will able to access package-private methods in MyClass

Answered By - dfritsi
Answer Checked By - Mary Flores (JavaFixing Volunteer)