2

I've noticed that when writing classes, it will often be my first impulse to make certain properties private since they wouldn't need to be referenced by other classes. Take for example a class that generates random strings.

public class RandomStringGenerator{
    private final Integer StringLength = 8;
    private final list<String> ValidStringCharacters = new list<String>{'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a'};

    public String getRandomString(){
        String ReturnString = '';
        for (Integer i = 0; i < StringLength; i++){
            Double CharacterToAdd = Math.random() * (ValidStringCharacters.size() - 1);
            ReturnString += ValidStringCharacters[(Integer) CharacterToAdd];
        }
        return ReturnString;
    }
}

The two variables StringLength and ValidStringCharacters could be made private since all of the other classes could ignore the length of the string and the characters that it is composed of, and just work with what they get. That's fine for all classes that is, except for the test class, which would want to make assertions based on those values to verify that the class is working properly.

@isTest
public class TestRandomStringGenerator{

    private static RandomStringGenerator rsg;

    static{
        rsg = new RandomStringGenerator();
    }

    public static @isTest void testStringLength(){
        system.assert(rsg.getRandomString().length() == rsg.StringLength);
    }

    public static @isTest void testStringCharacters(){
        Set<String> ValidCharacters = new Set<String>(rsg.ValidStringCharacters);
        String RandomString = rsg.getRandomString();
        for (Integer i = 0; i < RandomString.length(); i++){
            system.assert(ValidCharacters.contains(RandomString.substring(i,i+1)));
        }
    }

}

What is the preferred way to handle this situation? Should those private variables be made public just so that the test class can access them?

4
  • If your public methods are using those private variables, those declaration/assignment lines for private variables are covered.
    – ajinkyah
    Commented Feb 16, 2016 at 12:39
  • @ninja.hk True, if all you wanted to do was get the minimum amount of code coverage to be able to deploy the class, then you would be fine just calling the public method. The private variables are only being accessed for the assert statements.
    – martin
    Commented Feb 17, 2016 at 0:36
  • What I meant was, changing access modifiers just for coverage is wrong. And mostly it is done only for coverage as testing a functionality would not need changing access modifiers.
    – ajinkyah
    Commented Feb 17, 2016 at 5:18
  • @ninja.hk I agree that changing the access modifiers just for the sake of a test probably isn't the right approach. If you wouldn't mind sharing, what would you do in this situation to test the functionality of the class? The testStringLength and testStringCharacters methods in my question don't compile as written since they are referencing the private variables of the RandomStringGenerator class.
    – martin
    Commented Feb 17, 2016 at 5:38

2 Answers 2

3

You should definitely not be changing implementation details for the sake of tests.

If you need private members to be available in your tests then you should use the @TestVisible annotation.

There is however the deeper question of, does your test class need to use these variables at all? The answer is (most probably) no.

Think about what your class is meant to do and how you've encapsulated that behaviour.

Is it meant to generate a random string 8 characters in length? In which case you definitely don't want to compare against the StringLength variable, as if you change it your test will still pass even though your class no longer fits it's purpose.

If it is in fact meant to generate a string of StringLength characters, then why don't you give control of that variable to the caller? For example, by passing it in as a constructor argument. This will reduce bugs in other parts of your code that may be depending on this returning a string of a certain length and give you more flexibility in using it in the future.

A well-designed class should "never" require using @TestVisible.

3
  • Exactly the point! I see many people using @TestVisible without thinking about why that private variable/method was not accessed/called. If it is accessed, the line will be considered as covered.
    – ajinkyah
    Commented Feb 16, 2016 at 12:50
  • Thanks, it was @TestVisible that I was looking for. When writing this example, I had it in mind that this class could be used in multiple places, and would always produce output that conformed to the same constraints. If the number 8 was passed as a parameter each time the method was called, and if there needed to be a change where all random strings had to be 16 length instead, then all of those method calls would have to be updated, which would be a time consuming and error prone process.
    – martin
    Commented Feb 17, 2016 at 0:53
  • If the value is built to be changeable, then maybe it would be better to put it in Custom Metadata where it would be easier to update and could be accessed by both the Class and its test.
    – martin
    Commented Feb 17, 2016 at 0:57
2

You can use @TestVisible to access them in test class. I don't think it is best to make it visible for the test classes.

With this annotation, you don’t have to change the access modifiers of your methods and member variables to public if you want to access them in a test method. For example, if a private member variable isn’t supposed to be exposed to external classes but it should be accessible by a test method, you can add the TestVisible annotation to the variable definition.

Test Visible

You must log in to answer this question.

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