1

I am using MVP architecture for building my app. My presenter calls a DataManager which is responsible for getting data either from the network or a database. As I am using RxJava, I subscribe Observers in the Presenter and pass the appropriate state to UI. My service layer has the Android Context and it will also create an Exception of my own type which has a reference to the Context as well.

if (isNetworkConnected()) {
                    final Call<ServiceResponse<AppVersion>> call = mService.getAppVersion();
                    try {
                        final Response<ServiceResponse<AppVersion>> response = call.execute();
                        if (response.isSuccessful()) {
                            final ServiceResponse<AppVersion> serviceResponse = response.body(); response.body();
                            if (serviceResponse.isSuccess()) {
                                subscriber.onNext(serviceResponse.getData());
                            } else {
                                subscriber.onError(new CustomException(mContext, response.code(), response.message(), serviceResponse.getErrorList()));
                            }
                        } else {
                            subscriber.onError(new CustomException(mContext, response.code(), response.message(), response.errorBody().string()));
                        }
                    } catch (IOException e) {
                        e.printStackTrace();
                        subscriber.onError(e);
                    } finally {
                        subscriber.onCompleted();
                    }
                } else {
                    subscriber.onError(new NoInternetException());
                }

My CustomException also logs the crash in Crashlytics. When I am unit testing this code I am getting an exception from Crashlytics not being initialized. So I need to mock the static method logException of Crashlytics. But how should I pass this mock object as presenter doesn't take this object?

 public staticErrorType getErrorType(Throwable throwable) {
        //409: Not handled as its a conflict response code and comes in PUT/POST
        if (throwable instanceof IOException) {
            return ErrorType.NO_INTERNET;
        } else if (throwable instanceof CustomException) {
            final int errorCode = ((CustomException) throwable).mErrorCode;
            if (errorCode == 404) {
                return ErrorType.NOT_FOUND;
            } else if (errorCode == 401) {
                return ErrorType.UNAUTORISED;
            } else if (errorCode == 403) {
                return ErrorType.FORBIDDEN;
            } else if (errorCode == 500 || errorCode == 502) {
                return ErrorType.NO_SERVER_TRY_AGAIN;
            } else if (errorCode > 500 && errorCode < 599) {
                return ErrorType.NO_SERVER_TRY_LATER;
            } else if (errorCode == 1000) {
                return ErrorType.NO_COURSE_ENROLLED;
            } else if (errorCode == 1001) {
                return ErrorType.NO_COURSE_STARTED;
            }
        }
        if (throwable != null) {
            Crashlytics.logException(throwable);
        }
        return ErrorType.SOME_THING_WENT_WRONG;
    }
David Rawson
  • 20,912
  • 7
  • 88
  • 124
Akhil Dad
  • 1,804
  • 22
  • 35
  • This sounds like an issue with the design of your classes. The CustomException shouldn't itself log to Crashlyrics. Instead, it should be a responsibility of the subscriber to log if necessary. – David Rawson Dec 15 '16 at 05:55
  • @DavidRawson so custom exceptions should be subscribed by someone else or should I pass a custom logger class to exception which will log the exception to any platform say Crashlytics and will mock it for testing? – Akhil Dad Dec 15 '16 at 06:15
  • Thanks for editing - that makes the question clearer. A good solution is to use a wrapper class to wrap around the static method in Crashlytics. Then pass in the wrapper class as a dependency of the subscriber. See [this answer](http://stackoverflow.com/a/29841824/5241933) – David Rawson Dec 15 '16 at 07:24
  • @DavidRawson What should be done with the static methods of CustomException, it should be argument to the method itself, because the static method can't be removed. when this throwable is sent to Presenter, I need to know the kind of error in the presenter which I get via this method call, and according to that I need to tell view to render. So I can pass this CustomLogger class from presenter to static method or any other design pattern should be followed? – Akhil Dad Dec 15 '16 at 09:02
  • I'll update my answer – David Rawson Dec 15 '16 at 09:08

2 Answers2

7

What you have encountered is what some argue is a problem with static methods. The static method logException() seems harmless to add everywhere but it is actually hiding a real dependency. Since that dependency is now required for that class to run, it makes your class difficult to test.

A good solution would be to create a wrapper class with non-static methods. If we apply the solution in this answer to your code, it would look something Like this:

public class CrashLyticsWrapper { 
    public CrashLyticsWrapper() {} 

    public void logException(Throwable t) {
         CrashLytics.logException(t);
    }
}

This can then be passed in constructors as a dependency to the classes that need it. Since it is a non-static dependency and a class that you now control, you can easily mock it and verify against it if necessary.

A separate issue: exceptions are like value objects and in most cases should not have dependencies static or non-static. Doing something like this:

public CustomException extends RuntimeException() {
     public CustomException() {
          Crashlytics.logException(this); //don't do this!
     }
}

makes brittle and untestable code. You can see from the constructors that you override from sub-classing exception that there is a field for cause, a field for message, and that's about it. a good scope for an exception. If you need to add functionality that goes with an exception you should write separate error handlers that takes the exception as data or as a parameter. This is in accordance with the "single responsiblity" principle in SOLID.

Community
  • 1
  • 1
David Rawson
  • 20,912
  • 7
  • 88
  • 124
2

Another approach could be to mock the Crashlytics class containing static methods using powermock.

Testing a function that sends a log to Crashlytics could look like this

@RunWith(PowerMockRunner.class)
@PrepareForTest({Crashlytics.class})
@PowerMockIgnore({"javax.net.ssl.*"})
public class presenterTest{

  @Test public void testFunctionWithCrashlyticsCall() throws Exception{
      PowerMockito.mockStatic(Crashlytics.class);
      ...
      assertEquals(..)
   }

}

Documentation on this can be found here: https://github.com/powermock/powermock/wiki/Mockito#mocking-static-method

RWIL
  • 8,729
  • 1
  • 28
  • 37