Wednesday, October 21, 2015

Salesforce Debug logs with the Winter '16 Developer Console

I'm in the process of updating the FuseIT SFDC Explorer to use the Winter '16 (v35.0) APIs. One of the first things I've encountered are a number of changes with how debug logging works. These are somewhat documented in the Release Notes on Debugging.

In terms of actually integrating with Salesforce at the API level, TraceFlag no longer has a scopeId field to optionally represent the user being debugged. Instead it has a logType and DebugLevelId field. It's the latter that is particularly interesting.

Say you have two developers working in an Org. Ideally they would both be developing in separate orgs and then migrating changes into a common org, so let's say they are testing the integration of their changes.

  • Developer A starts the Developer Console.
  • A DebugLevel is created for Dev A with the DeveloperName 'SFDC_DevConsole'. Lets call it 7dl70000000000A, or DLA for short.
  • A TraceFlag is created for Dev A that references DLA. The TracedEntityId is the Developer A's User Id.
  • Developer B starts the Developer Console in the same org in their own session
  • You can't have two DebugLevel records with the same DeveloperName, so the Developer Console uses the existing DLA record. (See where I'm going with this?)
  • A TraceFlag is created for Dev B that references DLA. The TracedEntityId is Developer B's User Id.

So, what have we got? Both developers will see their logs in the Developer Console. However, they are sharing the same DebugLevel definition by default!

If either developer makes changes to the common SFDC_DevConsole debug levels it will affect the other developers logging for any future logs. If one developer deletes/removes the common DebugLevel it will cascade delete any associated TraceFlags. Hours of fun!

What can you do about this? You can add new DebugLevel records via the developer console. The one you have selected when you press Done will be sent back to the TraceFlag.

Thursday, October 8, 2015

Salesforce code quality test cases for Apex via static code analysis

Lets assume for a moment that your code coverage isn't a perfect 100% and there are some areas of your Apex that aren't exercised as part of the automated deployment test cases. If your suspension of disbelief goes that far, lets also assume that sometimes you add temporary code to aid in debugging those murky places. Something like:

System.assert(false, 'TEMP this assertion was only for debugging and will stop the execution so you can check the debug log etc...');

The general idea is that you can use this assertion to halt execution during manual testing. At which point it would be easy to inspect the debug log state or write out some useful state information in the assertion message. It's certainly not the most advanced debugging technique, but it is effective.

The main problem with the approach is what happens if you forget to take out that line. And then package the code base for deployment. Don't do that. It's not a good look.

Remember, the line in question may or may not be covered by the existing test cases. If it isn't it will sail right into the production managed package and then spring out and surprise a user at the most inopportune moment.

What if you could stop the package from proceeding based on the content of the Apex classes?

We need the poor man's static code analysis tool! And we need it to run as part of the packaging process.

Luckily, we already have Apex test cases that run as part of the packaging process. And we have the ApexClass sObject exposed to Apex which includes the Body of the classes within the current namespace. We just need some apex code that will scan through the Body of the apex classes and assert that they don't contain the problem string. Then add said test class to the package components. Something like:

@isTest
private class SampleCodeQualityTests {
 
 @isTest static void ensureThereAreNoTempAssertions() {
  string searchFor = '\'TEMP';

                // Exclude this class from the check. May also need to exclude based on namespace
  for(ApexClass ac : [Select Id, Name, Body From ApexClass where Name != 'SampleCodeQualityTests']) {
   integer index = ac.Body.indexOf(searchFor);
   if(index == -1) { continue; }

   integer lineNumber = lineNumberForIndex(ac.Body, index);
   string problemLine = extractLineByIndex(ac.body, index);
   System.assertEquals(-1, index, 'Apex class ' + ac.Name + '(' + ac.Id + ') contains the text ' + searchFor + ' at index ' + index + ' lineNumber:' + lineNumber + 
    '\n' + problemLine);
   
   //System.assert(!ac.Body.contains(searchFor), 'Apex class ' + ac.Name + '(' + ac.Id + ') contains the text ' + searchFor);
  }
 }

 private static integer lineNumberForIndex(string body, integer targetIndex) {
  integer lineNumber = body.substring(0, targetIndex).countMatches('\n') + 1;
  return lineNumber;
 }

 private static string extractLineByIndex(string body, integer targetIndex) {
  integer startOfLine = body.lastIndexOf('\n', targetIndex);
  integer endOfLine = body.indexOf('\n', targetIndex);

  string lineOfInterest = body.substring(startOfLine + 1, endOfLine);
  return lineOfInterest;
 }
 
}

Think of this more as a proof of concept. There are some obvious problems with it. For example, it isn't really parsing the Apex body. Just doing a dumb string search for a string that starts with TEMP. No consideration is being made for the context. It might be in comment. There are likely other issues around new line detection as well.

Still, it demonstrates the idea. Using Apex test cases to do static code analysis as part of the packaging and deployment steps.

Now if we could get to the SymbolTable in a testing context (i.e. no callout) there could be all sorts of interesting checks1:

  • Does each test method make at least one assertion?
  • Does each assertion include a message to the user?
  • Excessive lines of code in one Apex Class
  • Using System.debug statements - too many of these can be detrimental to performance.
  • Empty exception handling catch blocks
  • Unused local variables
  • Methods that aren't referenced by other code

Maybe having meaningless whitespace at the end of a code line or using somestring != null && somestring != '' rather than String.isEmpty(somestring) shouldn't be grounds to block packaging. You'll need to decide where the line gets drawn.

The other part of this type of testing is that it could form the basis of an automated org health check. Is the coverage percentage too low on a large class? Test fails. Trigger not bulkified? Test fails. An Apex class with 4000 lines of code that is padding for test coverage? Test fails!

You could easily drop them into an existing org to get a feel for the state of things before embarking on an modifications.

See also:


1. may or may not be actually possible with the SymbolTable alone.