Monday, December 16, 2013

Apex Test Methods - SeeAllData

One topic that I want to touch on is the SeeAllData attribute.  I thought to include it in my other post about test methods but decided to give this one it's own space.  First, let's get a definition on it:

***
Starting with Apex code saved using Salesforce.com API version 24.0 and later, test methods don’t have access by default to pre-existing data in the organization, such as standard objects, custom objects, and custom settings data, and can only access data that they create. However, objects that are used to manage your organization or metadata objects can still be accessed in your tests such as:
User
Profile
Organization
AsyncApexJob
CronTrigger
RecordType
ApexClass
ApexTrigger
ApexComponent
ApexPage
Whenever possible, you should create test data for each test. You can disable this restriction by annotating your test class or test method with the IsTest(SeeAllData=true) annotation.
Test code saved using Salesforce.com API version 23.0 or earlier continues to have access to all data in the organization and its data access is unchanged.

***

I've seen and heard many people claim that setting this attribute to true was a bad practice.  Go ahead and google it.  You'll find plenty of well-intentioned people telling you that you should never (or very rarely) set it to true and warn you like they were saving you from some kind of doom.

I say that there are plenty of reasons to use it and removing this from your tool set, puts you at a disadvantage.  Let's look at a couple examples of where I see some utility:

  • Existing org data
    • Volume
    • Quality
  • Custom settings
Setting SeeAllData to false (or accepting the current default), limits your test methods from existing org data. So for example, if you were to query for an existing account and set a field value in your test method, you'd end up with a query exception.  If you were the developer of a managed package, you'd probably be smart enough to recognize that this scenario and that your account doesn't exist in your customer's org, so you'd need to create a new account in your test methods.  In this case, SeeAllData = false, makes sense because you cannot assume that your test data exists in someone else's org.

If you were the developer of your company's internal Salesforce instance, your perspective and use cases are likely to be different. Imagine you've created a vf page that aggregates some account data.  For a page that does a lot of soql queries, you may be concerned about performance or governor limits.  If your org had millions of accounts, you could make your test methods try to simulate your account data volume, but it might be smarter to make use your existing data in your full sandbox also.  Running apex tests for both new and existing data, you'll be more confident in how your application behaves in production.  

Data quality is another place where setting SeeAllData = false may leave you vulnerable.  Let's say your org's history was something like this:
  1. In January, your users started using opportunities
  2. Later in March, they created a validation rule to make a field required on opportunities
  3. In June, you were asked to come in and add a trigger to activities to automatically update the related opportunity
If you were using SeeAllData = false, your test methods would create a new account and opportunity, create a related Task, and successfully assert that the Opportunity was updated by your new trigger.  Your test opportunity would have been created with all of the required data and your trigger would pass your unit tests.  However, the trigger would fail in production, perhaps unexpectedly, because you didn't know that there were January opportunities that had not been updated after the validation rule was introduced.  If you were using SeeAllData = true, you may have caught this data inconsistency and been prepared to handle the exception cleanly.

I think custom settings are another place where I'd look to use the SeeAllData.  In much of my apex code, I try to make the code flexible through a custom setting.  So, for example, if I am integrating via an http callout, I'd try to put the endpoint url in a custom setting.  Using SeeAllData = false, I could succesfully deploy to production without ever setting up the custom setting because my test methods create the custom setting. However, using SeeAllData = true, my test methods would fail because the custom setting had not been created in production.  This allows me to be sure that my dependency is in place before bringing users back into the system.  

So, there you have it.  SeeAllData = true can be your friend.  It's like bacon - plenty of people will tell you to avoid it, if you want to live.  But I say a little bit in your life can be delicious.
  

Deep Thoughts on Apex Test Methods

You're good enough.
You're smart enough.
You can write a good apex test method.

I just completed a major rewrite of all test methods for a client and while it was painful at times, I think it puts them in a position to extract some value from what was previously just a production deployment hurdle. Trust me, I'm not yet a full test-driven-development convert but I do believe that you can help your business automate some testing, and maybe even save some cash, if you take the time to think about your testing and apply it to your test methods.

You get better at the things you do over and over and writing good test methods is certainly something you can expect to have plenty of opportunity to practice.  There are some great resources out there to be sure you are repeating good habits. I'd start with Dan Appleman's Advanced Apex book as he has some great ideas for test class writing. Some other articles that I think are helpful and instructive are:

http://jessealtman.com/2013/09/proper-unit-test-structure-in-apex
http://wiki.developerforce.com/page/How_to_Write_Good_Unit_Tests

With this recent rewrite effort, some of the good practices I've incorporated are:

  • Moving test methods from functional classes into separate Test Classes
    • This allows you to decouple your tests from your functional classes, which excludes your tests' ability to reach private methods.  This will give you some flexibility with refactoring your code without being tied down by your test methods.
  • Asserting results
    • While you may achieve the 75% goal of code coverage, your tests will have little/no value if you are not actually checking expected versus actual results.  As a managed package developer, you'll also get flagged during the security review if you are not asserting your results. 
  • Centralizing and standardizing helper utilities
    • Like other apex you write, you should try to encapsulate where you can and use helpers to minimize the effort in testing various permutations of data against your code.
  • Testing negative scenarios
    • This is another area where you can get some value out of unit testing.  While it may take 80% of your effort to identify and code these, it's going to yield lots of value in improving your code and confidence in your code handling atypical scenarios.
  • Testing as an end user
    • Unfortunately as developers, we are system admins and almost everything we test works as expected because we don't have to deal with sharing or role hierarchy or object/field visibility. However, in the real world, our users are almost never system admins, so testing as a system admin makes no sense.

This is certainly not the complete list of best practices, but it's a good start.

As with other things in Salesforce, there is some room for improvement with the execution of unit testing in the application. In particular, I'm still frustrated by what Jeff Douglas calls the "black art".  Like Jeff, I've come across some peculiar behaviors that can be maddening.  For example, if you are testing a trigger and need to create a user and then test the dml, there's a pretty good chance you're going to get a mixed DML exception.  What is maddening, however, is that the error will not be caught in the force.com ide.  Oh, and you can deploy to production with this too!  The only thing keeping me sane was knowing that someone else noticed this too:



And aside from the nonsense of trying to estimate your code coverage in any of the tools out there, it would be nice if the test classes had the code coverage estimation like the functional classes for those of us separating them:


And don't get me started on the new test execution screens.  Aside from queuing your tests, they provide no value!

I think there is plenty to like about putting some effort into doing proper unit testing in Salesforce.  I'm sure if you build in the time to your sprints/plans, it will pay dividends in the long term.  Just be sure you approach this with a good sense of humor.