1

I need to write a test case for this code currently I have 63% coverage. I am new at this and am having trouble understanding how to target my code. As you can see in the image below I am failing with most of the forloops and .get lines. any help would be welcome.

Apex Class

public class SumOrdersOnAccount {
    @InvocableMethod
    public static void SumAccountOrders(List<Id> AccountIds)
    {
        for(Id record: AccountIds) {
            Account updateAccount = new Account(Id = record);

            updateAccount.Last_Season_Sales__c = null;
            updateAccount.Current_Season_Total_Orders__c = null;
            updateAccount.Total_Sales_2_Years_Ago__c = null;

            if ((date.today() >= date.newInstance(2017, 07, 01)) && (date.today() <= date.newInstance(2018, 06, 30))){
                for(AggregateResult result: [SELECT SUM(TotalAmount) Amt, AccountId Id FROM Order WHERE AccountId = :record AND (EffectiveDate >= :date.parse('07/01/2016')) AND (EffectiveDate <= :date.parse('06/30/2017')) GROUP BY AccountId]) {
                    updateAccount.Last_Season_Sales__c = (Decimal)result.get('Amt');
                }
                for(AggregateResult result: [SELECT SUM(TotalAmount) Amt, AccountId Id FROM Order WHERE AccountId = :record AND (EffectiveDate >= :date.parse('07/01/2017')) AND (EffectiveDate <= :date.parse('06/30/2018')) GROUP BY AccountId]) {
                    updateAccount.Current_Season_Total_Orders__c = (Decimal)result.get('Amt');
                }
            }

            if ((date.today() >= date.newInstance(2018, 07, 01)) && (date.today() <= date.newInstance(2019, 06, 30))){

                for(AggregateResult result: [SELECT SUM(TotalAmount) Amt, AccountId Id FROM Order WHERE AccountId = :record AND (EffectiveDate >= :date.parse('07/01/2017')) AND (EffectiveDate <= :date.parse('06/30/2018')) GROUP BY AccountId]) {
                    updateAccount.Last_Season_Sales__c = (Decimal)result.get('Amt');
                }
                for(AggregateResult result: [SELECT SUM(TotalAmount) Amt, AccountId Id FROM Order WHERE AccountId = :record AND (EffectiveDate >= :date.parse('07/01/2018')) AND (EffectiveDate <= :date.parse('06/30/2019')) GROUP BY AccountId]) {
                    updateAccount.Current_Season_Total_Orders__c = (Decimal)result.get('Amt');
                }
                for(AggregateResult result: [SELECT SUM(TotalAmount) Amt, AccountId Id FROM Order WHERE AccountId = :record AND (EffectiveDate >= :date.parse('07/01/2016')) AND (EffectiveDate <= :date.parse('06/30/2017')) GROUP BY AccountId]) {
                    updateAccount.Total_Sales_2_Years_Ago__c = (Decimal)result.get('Amt');
                }
            }

            update updateAccount;
        }
    }
}

Test

@isTest (seeAllData=false)

private class SumOrdersOnAccountTest {

    private static testMethod void doTest() {
        Test.startTest();

        Account a = new Account(Name = 'Test Co');
        insert a;
        Order o = new Order(AccountId = a.id, EffectiveDate=system.today(), Status = 'Draft', Inv_Nbr__c = 99999);
        insert o;



        List<Id> myIds = new List<Id>();
        myIds.add(a.Id);

        SumOrdersOnAccount.SumAccountOrders(myIds);



        Test.stopTest(); 
    }
}

Areas of failed coverage:

enter image description here

5
  • 1
    Please edit your original post to explain what your code is supposed to do, how it is executed, and what part of your unit test is not working. Sep 8, 2017 at 18:52
  • As a side note, there is likely a way for you to improve this code such that you would only need 1 query (instead of basically repeating the same query 4 additional times). As for how to do that, that would be another question...but making a formula field to determine which fiscal year an Order was part of should be relatively simple.
    – Derek F
    Sep 8, 2017 at 18:58
  • I updated the question to show more info, thanks for the suggestion @David Cheng Sep 8, 2017 at 19:03
  • @DerekF - I am working on learning how to do that in Apex, I recently came over from JavaScript so hopefully that will translate well for me. Sep 8, 2017 at 19:05
  • @DerekF, I think what you saw is it. Could you give me some pointers on how to build that? thanks Sep 8, 2017 at 19:15

1 Answer 1

0

It looks like your unit test is pretty close to working the way you expect it to be. You're already taking care of one of the important parts of a unit test, calling the method you want to test.

You're missing other important components of a unit test. Unit tests aren't very useful if you don't make assertions (e.g. system.assertEquals(valueIExpected, valueIReceived, 'Thing xyz didn\'t work as expected. Field a__c should be a rollup of values from Object B');.

The part you seem to be stuck on, however, is the test setup. You only get coverage for code that is executed as part of a unit test. If parts of your code aren't covered, then you either aren't calling that method, or, as is the case here, the data setup you're doing for your test is inadequate.

Let's take a look at the first line of code that you're missing coverage for.

if ((date.today() >= date.newInstance(2017, 07, 01)) && (date.today() <= date.newInstance(2018, 06, 30))){
   for(AggregateResult result: [SELECT SUM(TotalAmount) Amt, AccountId Id FROM Order WHERE AccountId = :record AND (EffectiveDate >= :date.parse('07/01/2016')) AND (EffectiveDate <= :date.parse('06/30/2017')) GROUP BY AccountId]) {
     updateAccount.Last_Season_Sales__c = (Decimal)result.get('Amt'); // This line is not covered
  }

Now, let's take a look at your test setup

Account a = new Account(Name = 'Test Co');
insert a;
Order o = new Order(AccountId = a.id, EffectiveDate=system.today(), Status = 'Draft', Inv_Nbr__c = 99999);
insert o;

Since date.today() is 2017-9-8, the one Order record you're inserting does not meet the criteria for the query that looks for the Orders for FY (fiscal year) 2016. Since your query is returning 0 rows, you never actually get into that particular for loop. Therefore, the code inside the loop is not executed, and hence, it isn't covered.

Fixing that particular issue would be as simple as creating and inserting another Order record where EffectiveDate was sometime in the previous fiscal year. You could take care of that by simply setting the new Order's EffectiveDate to date.today().addYears(-1)...but you may (or may not) run into issues at your boundary conditions (the day of and day before/after the start of the new fiscal year).

Getting coverage for your other uncovered code is currently impossible because you've hard-coded the days you're allowed to execute the other block (the one to get you the performance for this year + the past 2 FYs instead of just this year + the last FY), and are using date.today() as the other basis for your comparison. This is a great example of why hard-coding things is generally a bad idea.

Code that is hard or impossible to test is a sign that you need to re-think your code (up to completely re-writing it using a different approach).

One other thing of note is that you have queries inside of a loop (in the code being tested). Queries inside of loops are bad. If I were reviewing this code for promotion from a sandbox to production (or in this case, even between sandboxes), I would fail it for having queries in loops (and prevent the code from being promoted).

Fixing those problems, however, is outside of the scope of this question. Like I mentioned in the comments, I think that creating a formula field that tells you which FY an Order is in would help you arrive at a better implementation of your SumOrdersOnAccount class. I encourage you to try to work out that formula field yourself. If you get stuck, use the search box in the upper right corner of this site to see if someone else has asked a similar question (and ask a new question if you don't find anything).

Once you have that taken care of, try to rewrite your class from scratch, and make use of that FY formula field to get down to a single query. Again, try to do it yourself, then search for similar questions if you're stuck, then ask another question if you don't find any.

+edit:

Forgot to mention that aggregate functions can operate with null values, but they do not coalesce null values. As is, updateAccount.Current_Season_Total_Orders__c = (Decimal)result.get('Amt'); will likely result in null being put into Current_Season_Total_Orders__c (because your test setup does not specify Amount for any Order).

The quick and simple 'null coalescing' that I find myself doing would look like this:

// The (boolean test) ? <expression if true> : <expression if false>; syntax is
//   called a ternary operator.
//Here, it's equivalent to if(condition){ Current_Season_Total_Orders__c = value1;}
//   else{ Current_Season_Total_Orders__c = value2; }, but
//   requires a little less typing.
// I use if/else for most things, but I find using the ternary is a good fit for 
//   null coalescing.
updateAccount.Current_Season_Total_Orders__c = (Decimal)result.get('Amt') == null ? 0 : (Decimal)result.get('Amt');

This also brings me to another point that I forgot to mention. A class method can have more than one unit test that exercises it. For all but the simplest and most trivial of methods, I'd go so far as to say you should have more than one test for it.

I personally try to break up unit tests based on the different situations that I can dream up. For the class you're testing, an example of the unit tests I would/could write would be

  • what happens when an Account has no Orders
  • what happens when an Account has one Order
  • what happens when an Account has more than one Order
  • what happens when there are Orders associated to different Accounts, and you only update one of the Accounts
  • what happens when none of the Orders associated to an Account have a value in Amount?
  • what happens when some of the Orders associated to an Account have a value in Amount, but others don't?
  • what happens when an Order sits on the first day of a Fiscal Year?
  • what happens when an Order sits on the last day of a Fiscal Year?
  • etc...

Basically, test the boundary conditions. Yes, it's a lot of work to write an appropriate number of unit tests. However, using that mindset should mean that you'll never have code coverage issues. If you make appropriate assertions in your tests as well, passing tests will give you confidence that your code is correct and behaves as expected. Tests that continue to pass in the future also give you confidence that new code/triggers/objects/packages you deploy doesn't break your existing code.

1
  • Thank you for your help! You were right on with your analysis and you gave me some great pointers. I will work on your suggestions. Thanks again. Sep 8, 2017 at 20:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.