1

I'm new to Salesforce. Creating Test class for a trigger isn't as tough as creating test class for simple class that only has a dml and a query. The line "clInfo.GetAll();" is giving me 100% code coverage but I am not sure if this is the right way of testing this class. My test clss doesn't have the inserting of records, the query of these objects nor assertions but it gives 100% with just "clInfo.GetAll();". Is it the right way?

@RestResource(urlMapping='/info')
global with sharing class clInfo{

@HttpGet 
global static Wrapper GetAll(){

    Wrapper wrap = new Wrapper('', '', null, null);

    String orgId = UserInfo.getOrganizationId(); // retrieve org ID
    String orgName = UserInfo.getOrganizationName(); // retrieve org Name

    datetime dt = System.now()-1; // use to filter results for System Overview query

    // query for the Installed Packages and System Overview
    List<Installed_Package__c> packageList = [SELECT Name, Version__c, License__c, Date_Installed__c, Date_Updated__c FROM Installed_Package__c];
    List<System_Overview__c> sysOv = [SELECT Max_API__c, Requested_API__c, Usable_API__c, Total_Storage__c, Used_Storage__c, Remaining_Storage__c, Retrieve_DateTime__c FROM System_Overview__c where Retrieve_DateTime__c > :dt];

    wrap.packages=packageList;
    wrap.system_overview=sysOv;
    wrap.orgId = orgId;
    wrap.orgName = orgName;

    return wrap;

}

global class Wrapper{

    public String orgId{get;set;}
    public String orgName{get;set;}

    public List<Installed_Package__c> packages{get; set;}
    public List<System_Overview__c> system_overview{get; set;}


}}

Here's my test class:

@isTest
global class GetSysOvClassTest {

@isTest static void testHere() {

    clInfo.GetAll();
}}

I have updated my test class into this:

@isTest static void testHere() {
    //created records here
    //inserted the records here
    datetime dt = System.now()-1;
    List<Installed_Package__c> SFpackRetrieve = [SELECT ID FROM Installed_Package__c]; //query the records
    List<System_Overview__c> SFsysOvRetrieve = [SELECT ID FROM System_Overview__c WHERE Retrieve_DateTime__c > :dt]; //query the records
    clInfo.Wrapper testWrap = new clInfo.Wrapper('00D6F000002KtVEUA0 ', 'Org Client', SFpackRetrieve, SFsysOvRetrieve);
    System.assertEquals(2, SFpackRetrieve.size());
    System.assertEquals(1, SFsysOvRetrieve.size());
}}

I removed the "clInfo.GetAll();" line in the test class and now I have 42% code coverage. Will it be alright to add this line (clInfo.GetAll();) once again? My GetAll() method isn't covered. I add that line, it goes 100%.

1 Answer 1

2

This test yields 100% code coverage, but it doesn't actually test anything. Code coverage is a very weak metric: it shows only that the lines of code are executed in a test context and don't throw an unhandled exception, not that they do anything either correct or useful. Here, it can be deceptive if you're new to the underlying concepts.

Another way to answer your question is to ask "What does this test tell me about how my code is behaving?" In this case, it tells you exactly one thing: when the code is called in a test context containing no data at all (no records of Installed_Package__c or System_Overview__c), it does not throw an exception.

Since your code will in the real world mostly not be executing in such an environment, what this test is telling you is worth very little. It makes no meaningful claims about your code's behavior and its correctness.

To turn this into a real unit test, you would need to seed some plausible, but invented, data in your test context by creating and inserting records for those two objects, or using Test.loadData() to load them from a Static Resource.

Then, you'd call GetAll(). And once you do call it, you'd write assertions against the returned list of Wrapper objects to validate that the return values match the seeded data that you created. That would demonstrate that your code is behaving correctly.

It's also a good idea to do a null test like this, where your code has to correctly handle the situation where no data is available. But even here, you should be writing some assertions to show that your code is returning a reasonable value when it has no data available to it.

How to Write Good Unit Tests is a useful resource for this type of problem.

2
  • Hi @david-reed , I have udpated my test class above. Commented Aug 27, 2018 at 4:39
  • You don't really need to test the getter and setter methods in your wrapper class (although you can), because they're just system-generated. Yes, you should definitely call getAll() and then make assertions about the wrapper it returns.
    – David Reed
    Commented Aug 27, 2018 at 11:08

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .