Tests for Salesforce controller concerning candidates and notes

Question

I have this controller and some test cases. Any corrections/advice are appreciated to make them better.

public with sharing class MTX_MatrixDetailViewController {

    public transient String activities {get;set;}

    public MTX_MatrixDetailViewController(){
        
        String candidateId =  ApexPages.currentPage().getParameters().get('id');

         Map<String, String> filters = new Map<String, String>();
        
        List<LightningActivity> actList = LightningActivityAccessor.getLightningActivitiesByType('Contact', candidateId, filters, 0);
                
        //sort activities
        actList = LightningActivityAccessor.sort(actList,'date','DESC');

        activities = JSON.serialize(actList);
        
    }
    
    @AuraEnabled 
    public static MTX_MatrixWrapper fetchCandidateDetails(String candidateId, String matrixId){

        MTX_MatrixWrapper wrapper = new MTX_MatrixWrapper();

        wrapper.candidateRecord = MTX_MatrixAccessor.getCandidatesById(new List<String>{candidateId})[0];

        wrapper.matrixCandidate = MTX_MatrixAccessor.getMatrixCandidatesById(new List<String>{matrixId})[0];

        wrapper.currentMatrix = String.isNotBlank(wrapper.matrixCandidate.Matrix__c) ? MTX_MatrixAccessor.getMatrixById(wrapper.matrixCandidate.Matrix__c) : new MTX_Matrix__c( Name = Label.MTX_Candidates_Shared_with_Me );

        wrapper.preferredMobile = MTX_MatrixAccessor.getPreferedMobileForCandidate(wrapper.candidateRecord.Master_People__c);

        wrapper.candidates = MTX_MatrixAccessor.getMatrixCandidatesById(new List<String>{matrixId});
        
       List<Matrix_User_Setting__c> userSettings = MTX_MatrixAccessor.getSettingsForCurrentUser(UserInfo.getUserId());

        wrapper.userSettings = userSettings.isEmpty() == false ? userSettings[0] : MTX_MatrixService.createMatrixUserSetting(UserInfo.getUserId());
        /*** XEngine ***/
        XEngineUtils.postMatrixCandidatesEvent(wrapper.candidates, XEngineUtils.MTX_CAND_CANDIDATE_DETAILS);
        /*** XEngine ***/
        
        return wrapper;
    } 
    
    @AuraEnabled 
    public static MTX_Matrix_Note__c getNewMatrixNote(String candidateId){
        
        return new MTX_Matrix_Note__c(Candidate__c = candidateId);
    }
    
    @AuraEnabled 
    public static MTX_Matrix_Note__c saveNote(MTX_Matrix_Note__c note){
        
        return MTX_MatrixService.saveNote(note);
    }
    
    @AuraEnabled 
    public static MTX_Matrix_Note__c getNoteById(Id noteId){
        
/*** XEngine ***/
        XEngineUtils.postMatrixNoteEvent(noteId, XEngineUtils.NOTE_VIEW);
        /*** XEngine ***/

        return MTX_MatrixAccessor.getMatrixNoteById(noteId);
    }
    
    @AuraEnabled 
    public static void deleteSelectedNote(MTX_Matrix_Note__c note){
        
        MTX_MatrixService.deleteCandidateNote(note);
    }

}

Here are my test classes:

@isTest
public class MTX_MatrixDetailViewControllerTest {
    
    
    //create test data 
    @testSetup static void setup() {
        
        //setup FO full users (not inserted yet)
        List<User> matrixUsersToGivePermission = MatrixTestFactory.createMatrixUsers(2);
        
        //give FO full users matrix permission set (will insert the users)
        matrixUsersToGivePermission = MatrixTestFactory.giveMatrixPermission(matrixUsersToGivePermission);
        
        //setup spotlight
        MatrixTestFactory.setupEliseConnection();
        
    }
    
    
    static testMethod void gettingnewNote{
        List<Contact> candidates = MatrixTestFactory.getCandidates(); 
        List<User> matrixUsers = MatrixTestFactory.getMatrixUsers();
        Test.startTest();
        //MTX_Matrix_Note__c nt = new MTX_Matrix_Note__c();
        System.runAs(matrixUsers[0]) {
            MTX_MatrixService.getNewMatrixNote( candidates[0].Id );            
        }
        Test.stopTest();        
    } 
    
    static testMethod void savingtheNote{
        //create a method for for getting notes in MatrixTestFactory
        List<MTX_Matrix_Note__c> notes = MatrixTestFactory.getNotes();
        List<User> matrixUsers = MatrixTestFactory.getMatrixUsers();
        Test.startTest();
        //MTX_Matrix_Note__c nt = new MTX_Matrix_Note__c();
        System.runAs(matrixUsers[0]) {
            MTX_MatrixService.saveNote(notes[0] );            
        }   
        Test.stopTest();
    }
    
    static testMethod void deletingspecificNote{
        List<User> matrixUsers = MatrixTestFactory.getMatrixUsers();
        //create a method for for getting notes in MatrixTestFactory
        List<MTX_Matrix_Note__c> notes = MatrixTestFactory.getNotes();
        Test.startTest();
        //MTX_Matrix_Note__c nt = new MTX_Matrix_Note__c();
        System.runAs(matrixUsers[0]) {
            MTX_MatrixService.deleteCandidateNote(notes[0]);            
        }
        Test.stopTest();
    }
    static testMethod void retrievingNoteiD{
        List<User> matrixUsers = MatrixTestFactory.getMatrixUsers();
        //create a method for for getting notes in MatrixTestFactory
        List<MTX_Matrix_Note__c> notes = MatrixTestFactory.getNotes();
        Test.startTest();
        //MTX_Matrix_Note__c nt = new MTX_Matrix_Note__c();
        System.runAs(matrixUsers[0]) {
            MTX_MatrixService.deleteCandidateNote( notes[0]);            
        }
        Test.stopTest();
    }
    
}

Side note: Matrix Test Factory creates my test data, and when I call  MTX_MatrixService.deleteCandidateNote, MTX_MatrixService.saveNote, MTX_MatrixService.getNewMatrixNote and MTX_MatrixAccessor.getMatrixNoteById, they are defined as

public static MTX_Matrix_Note__c saveNote(MTX_Matrix_Note__c note){

        boolean addNote;
        if(note.Id != null) { addNote = true; } else { addNote = false; }

        upsert note;

        /*** XEngine ***/
        if(!addNote){
            XEngineUtils.postMatrixNoteEvent(note.Id, XEngineUtils.NOTE_ADD);
        } else {
            XEngineUtils.postMatrixNoteEvent(note.Id, XEngineUtils.NOTE_UPDATE);
        }
        /*** XEngine ***/

        return note;
    }

    public static void deleteCandidateNote(MTX_Matrix_Note__c note){

        string matrixNoteJson = XEngineUtils.getMatrixNoteJson(note, XEngineUtils.NOTE_DELETE);

        delete note;

         /*** XEngine ***/
        XEngineUtils.postEventJson(matrixNoteJson);
        /*** XEngine ***/
    }
/*retrieves single matrix note content by note id*/
    public static MTX_Matrix_Note__c getMatrixNoteById(Id noteId){

        List<MTX_Matrix_Note__c> notes = new List<MTX_Matrix_Note__c>();

        notes = [SELECT Id, Note__c, CreatedById, CreatedBy.Name, LastModifiedDate FROM MTX_Matrix_Note__c WHERE Id =: noteId];

        return notes.isEmpty() == false ? notes[0] : null;
    }
```

Answer 1

Your unit tests contain no assertions about the behavior of your code, so they prove nothing other than that the code does not crash in this specific situation. These are commonly referred to as smoke tests.

There are three critical elements for any unit test, even before you start thinking about the overall coverage of your application's code paths:

1. Control environment and set up test data.
2. Execute functionality.
3. Validate results.

You are only doing (1) and (2) so far.

I'd recommend reviewing Salesforce Stack Exchange's canonical QA, How do I write an Apex unit test?, for more information and resources. In particular, you should definitely complete Unit Testing on the Lightning Platform on Trailhead.

Answer 2

Here are some code comments for you

For your constructor here are some changes I'd suggest and the reasoning behind them

public transient String activities {get;set;}
public static final String CONTACT = 'Contact';
public MTX_MatrixDetailViewController(){

    String candidateId =  ApexPages.currentPage().getParameters().get('id'); // change this type to an Id not a String.        
    //change to this and delete the filters declaration. Also, check if candidateId is not null.. maybe the id was not passed 
    if (String.isNotBlank(candidateId)){
        List<LightningActivity> actList = LightningActivityAccessor.getLightningActivitiesByType(
            CONTACT, 
            candidateId,  
            new Map<String, String>() , 
            0); 

        //sort activities
        actList = LightningActivityAccessor.sort(actList,'date','DESC');
        activities = JSON.serialize(actList);

    }
}

Here are some comments on the fetchCandidateDetails. Note, that I'm suggesting a couple of things - error checking and some stylistic stuff. This looks great otherwise!

@AuraEnabled 
public static MTX_MatrixWrapper fetchCandidateDetails(String candidateId, String matrixId){

    // you ought to check to make sure this data is valid. If you're doing this in the caller already, you can disregard.
    if(String.isBlank(candidateId) || String.isBlank(matrixId)){
        throw new MTX_Exception('Insuffinent Canditate Details'); 
    }

    MTX_MatrixWrapper wrapper = new MTX_MatrixWrapper();

    wrapper.candidateRecord = MTX_MatrixAccessor.getCandidatesById(new List<String>{candidateId})[0];

    wrapper.matrixCandidate = MTX_MatrixAccessor.getMatrixCandidatesById(new List<String>{matrixId})[0];

    wrapper.currentMatrix = String.isNotBlank(wrapper.matrixCandidate.Matrix__c) ? 
        MTX_MatrixAccessor.getMatrixById(wrapper.matrixCandidate.Matrix__c) : 
        new MTX_Matrix__c( Name = Label.MTX_Candidates_Shared_with_Me );  // moved to different lines. easier to read

    wrapper.preferredMobile = MTX_MatrixAccessor.getPreferedMobileForCandidate(wrapper.candidateRecord.Master_People__c);

    wrapper.candidates = MTX_MatrixAccessor.getMatrixCandidatesById(new List<String>{matrixId});

   List<Matrix_User_Setting__c> userSettings = MTX_MatrixAccessor.getSettingsForCurrentUser(UserInfo.getUserId());

    wrapper.userSettings = userSettings.isEmpty() == false ? 
        userSettings[0] : 
        MTX_MatrixService.createMatrixUserSetting(UserInfo.getUserId());  // moved to different lines. easier to read

    /*** XEngine ***/
    XEngineUtils.postMatrixCandidatesEvent(wrapper.candidates, XEngineUtils.MTX_CAND_CANDIDATE_DETAILS);
    /*** XEngine ***/

    return wrapper;
} 

The rest of your code looks good. Your Test code has some concerns. You're running tests for code coverage but you're not asserting anything. Also, you're only testing the happy path from what I gather.

For assertions - assert behavior that you expect to happen for your test set up.

For happy paths, test for other paths - negative, bulk, and boundary...So, for example you have a method getNewMatrixNote(candidateId) and your test case method is gettingnewNote(). What if I had a candidateId that was bad? That's why you need to have negative and boundary tests.