I'll start by noting that your test method is lacking in assertions. The one assertion that you do have is useless (might as well be System.assertEquals(1,1)
).
Coverage is always a secondary concern in unit testing.
That aside, It looks like you're starting to come to a realization that I myself have had as well.
Testing things that you can't (easily) control is hard.
The code in your doGet()
method isn't really prone to throwing an exception of any flavor. About the only exception that you could force to occur is a System.LimitException
by executing some number of queries in a loop in your test method so that when you call doGet()
, the first or second query will put you over the limit.
The only problem with that approach is that System.LimitException
is un-catchable.
There is a better way
Dependency Injection (DI for short) combined with apex interfaces are a really powerful team, especially for unit testing.
I think that being able to use DI requires a shift in perspective (compared to the perspective that led to the development of your rest service). Taking a few of the principles from SOLID object-oriented design, specifically the Single Responsibility Principle and Dependency Inversion principle, should help get you into the right mindset.
Breaking up your REST service
Right now, your rest service is violating the Single Responsibility Principle. It currently has 3 responsibilities: handling the REST request/response, querying for data, and building the list of data to be returned.
I find that it doesn't always make sense to break everything down to a single responsibility, but breaking your REST service into two separate classes is completely reasonable.
An example of breaking things into two classes could be this:
public class RestWorker{
// Passing the RestRequest/Response instances into this method is an example of
// Dependency Injection.
// When using an inner class of another class, we just need to use
// <outer class name>.<inner class name>
public List<RestObjectClass.WrapperClass> doWork(RestRequest req, RestResponse res){
List<RestObjectClass.WrapperClass> resultList = new List<WrapperClass>();
// Notice that we don't have any try/catch here.
// That's because this code has a 0% chance of throwing a catchable
// exception.
// That means that a try/catch here would accomplish precisely nothing.
Map<Id, User> contactToUser = new Map<Id, User>();
for (User u : [select Id, FirstName, LastName, UserName, Email,ContactId from User where ContactId != null ] ) {
contactToUser.put(u.ContactId, u);
}
for (Contact c : [select Id, AccountId, Lastname, Firstname, Email, Account.Id, Account.Name, Account.Phone from Contact where Id in :contactToUser.keySet()]) {
resultList.add(new RestObjectClass.WrapperClass(contactToUser.get(c.Id), c.Account, c));
}
return resultList;
}
and
@RestResource(urlMapping='/resturl/*')
Global class RestObjectClass {
@HttpGet
global static List<WrapperClass> doGet(){
List<WrapperClass> listmw = new List<WrapperClass>();
// The try/catch block remains in this class.
// Even though we know that RestWorker.doWork() can't possibly throw a catchable
// exception, it's best to pretend that we don't know anything about
// RestWorker.doWork().
// Treating doWork() as a black-box (something that we can't see into) adheres
// to the Dependency Inversion principle
try {
RestWorker worker = new RestWorker();
listmw = worker.doWork(RestContext.request, RestContext.response);
}catch(Exception ee) {
System.debug('The following exception has occurred: ' + ee.getMessage());
}
return listmw;
}
Global class WrapperClass {
User us;
Account acc;
Contact con;
Public WrapperClass (User us, Account acc, Contact con){
this.us = us;
this.acc = acc;
this.con = con;
}
}
}
Now, simply breaking your REST service up into two classes didn't accomplish too much beyond making you type some more.
This was merely preparation for the next logical step, which is to start using apex interfaces.
An interface is a contract. It tells you the methods that will exist, and the type of data that you will get back from a class that implements that interface. A class that implements an interface can have extra methods beyond what the interface specifies, but it must not lack any methods specified by the interface.
The example now expands to be 3 separate classes
// Interfaces don't have attributes, they only specify method signatures
public interface IRestWork{
// This is a method signature.
// It's just the access modifier, the return type, the name of the method,
// and the parameter list
// The semicolon at the end of the signature is required
public List<RestObjectClass.WrapperClass> doWork(RestRequest req, RestResponse res);
}
and
// Implementing the interface is as simple as adding 'implements <interface>'
// to the end of the class name (well, that and implementing the specified methods).
public class RestWorker implements IRestWork{
// Passing the RestRequest/Response instances into this method is an example of
// Dependency Injection.
// When using an inner class of another class, we just need to use
// <outer class name>.<inner class name>
// This is the only method that needs to be implemented to satisfy being
// part of the IRestWork interface.
// Since we already did this, there's no additional work to do here.
public List<RestObjectClass.WrapperClass> doWork(RestRequest req, RestResponse res){
List<RestObjectClass.WrapperClass> resultList = new List<RestObjectClass.WrapperClass>();
// Notice that we don't have any try/catch here.
// That's because this code has a 0% chance of throwing a catchable
// exception.
// That means that a try/catch here would accomplish precisely nothing.
Map<Id, User> contactToUser = new Map<Id, User>();
for (User u : [select Id, FirstName, LastName, UserName, Email,ContactId from User where ContactId != null ] ) {
contactToUser.put(u.ContactId, u);
}
for (Contact c : [select Id, AccountId, Lastname, Firstname, Email, Account.Id, Account.Name, Account.Phone from Contact where Id in :contactToUser.keySet()]) {
resultList.add(new RestObjectClass.WrapperClass(contactToUser.get(c.Id), c.Account, c));
}
return resultList;
}
and
@RestResource(urlMapping='/resturl/*')
Global class RestObjectClass {
// This additional line is instrumental to accomplishing our end goal
// of testing the catch block.
// This gives us a form of Dependency Injection, which gives us control.
public static IRestWork worker = new RestWorker();
@HttpGet
global static List<WrapperClass> doGet(){
List<WrapperClass> listmw = new List<WrapperClass>();
// The try/catch block remains in this class.
// Even though we know that RestWorker.doWork() can't possibly throw a catchable
// exception, it's best to pretend that we don't know anything about
// RestWorker.doWork().
// Treating doWork() as a black-box (something that we can't see into) adheres
// to the Dependency Inversion principle
try {
// The declaration of worker was moved outside of the method
// (for dependency injection reasons)
// Being a static method, we can simply just access the static variable
listmw = worker.doWork(RestContext.request, RestContext.response);
}catch(Exception ee) {
System.debug('The following exception has occurred: ' + ee.getMessage());
}
return listmw;
}
Global class WrapperClass {
User us;
Account acc;
Contact con;
Public WrapperClass (User us, Account acc, Contact con){
this.us = us;
this.acc = acc;
this.con = con;
}
}
}
That's great, but I still don't see how that helps in testing a try/catch block
You're right, just calling the REST service as-is won't result in an exception. It does, however, provide us with everything we need to test the try/catch.
The final piece of the puzzle lies in your test class.
In your test class, you can make 2 inner class like this
// Making our own exception type here is required, because we can't
// simply say 'throw new Exception()' (that code wouldn't compile)
private class TestRestException extends Exception{
}
public class TestRestWorker implements IRestWork{
public List<Wrapper> doWork(RestRequest req, RestResponse res){
List<Wrapper> result = new List<Wrapper>();
throw new TestRestException();
return result;
}
}
Finally, in your test, sometime before calling RestObjectClass.doGet()
, insert the following line
RestObjectClass.worker = new TestRestWorker();
Doing so will guarantee that you get an exception when you call RestObjectClass.doGet()
, meaning that you'll be exercising the functionality of the catch block.
Some final notes
The try/catch that you have in your test class is bad. If you get an exception that makes it all the way up to your test method, then your test should fail.
Code coverage is only a secondary goal of unit testing. The primary goal is to ensure that your code behaves the way that you expect it to. To do that, you need to use System.assertEquals()
and System.assertNotEquals()
.
Without assertions, you're only doing a 'smoke test' to see if your code is more sturdy than a house made of straw (i.e. that things don't explode when you run the code). Your code could say that 2+2 = fish, which is obviously wrong, but without assertions you don't know if your code is saying that 2+2 = fish, or if 2+2 = 4.
Your REST service is returning a List<Wrapper>
, you need to be making assertions against that List to make sure that it contains the data you think it should.
Building on that, you will need at least one more test method in your test class for RestObjectClass
. One that tests what happens when you run things normally, and a separate test method that tests what happens when you force the exception to occur.
Of course, making additional classes means that you'll need to make additional tests as well. Following SOLID principles should hopefully mean that the extra classes that you write are relatively small (and thus quick/easy to test).