Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing Application Factory without existing CMDT config data #18

Open
torynet opened this issue Jan 17, 2019 · 10 comments
Open

Testing Application Factory without existing CMDT config data #18

torynet opened this issue Jan 17, 2019 · 10 comments

Comments

@torynet
Copy link

torynet commented Jan 17, 2019

John,

In working on creating unit tests for the Application Factory, my team stumbled upon a difficulty in trying to test it. As far as I can tell, there is no easy way to give it "mock" CMDT records with which to work, in writing unit tests for the application factory code itself.

I handled the same issue when I wrote my own SObject type to SObject type binding capability that I have described to you previously. In that instance I had to make my own binding provider. When I did that, I had it pull the important mapping data from the CMDT into an @testvisible private static map property.

    @testVisible
    private static Map<String,Map<SObjectType,SObjectType>> SObjectTypeBindingMap{
        get{
            if(SObjectTypeBindingMap == null){
                SObjectTypeBindingMap = new Map<String,Map<SObjectType,SObjectType>>();
                List<base_SObjectTypeBinding__mdt> sotBindings = [SELECT Id
                                                                    , DeveloperName
                                                                    , Purpose__c
                                                                    , BindingSObjectType__c
                                                                    , BindingSObjectType__r.QualifiedApiName
                                                                    , BoundSObjectType__c
                                                                    , BoundSObjectType__r.QualifiedApiName
                                                                    FROM base_SObjectTypeBinding__mdt];
                for(base_SObjectTypeBinding__mdt sotBinding : sotBindings){
                    if(!SObjectTypeBindingMap.containsKey(sotBinding.Purpose__c)){
                        SObjectTypeBindingMap.put(sotBinding.Purpose__c,new Map<SObjectType,SObjectType>());
                    }
                    SObjectTypeBindingMap.get(sotBinding.Purpose__c).put(sObjectTypesMap.get(sotBinding.BindingSObjectType__r.QualifiedApiName),sObjectTypesMap.get(sotBinding.BoundSObjectType__r.QualifiedApiName));
                }
            }
            return SObjectTypeBindingMap;
        }
        set;
    }

This way I could easily "mock" the configuration data for unit tests, by just setting that property.

I was thinking of modifying your other application factory classes to do something similar. Before I did that, I thought it would be a prudent to ask how you have dealt with this? I want to make sure I'm not just missing something obvious.

Thanks, as always, for your time and attention.

@ImJohnMDaniel
Copy link
Collaborator

Hey @torynet -- thinks for reaching out.

If I understand things correctly, you are trying to test the App Factory itself? If that is the case, my question would be why?

Most (if not all) of that code will be exercised when you setup a selector or domain class and test that class.

I am not sure that I understand the value of mocking the binding MDT records when the regular application records are visible to unit tests anyway.

Am I missing something?

@torynet
Copy link
Author

torynet commented Jan 17, 2019

Thanks for taking the time to respond John.

I guess that's just normally how we do things. We test all units. We don't consider whether or not they might be run by the tests of the layers above them. Do you normally only test from the topmost layer? By that logic is there any need for any tests in any of the underlying libraries like Force DI, fflib, etc?

The reason for creating test config data (which is really a more accurate way of describing it than my previous use of the word "mock"), is that it allows you to test possible configurations rather than just testing the actual configuration you have created. When you create a configurable system, IMHO it's important to test the capability of that system, rather than just individual configurations you may have made that utilize it. Inevitably I will eventually have developers making changes to these classes, and I want the benefits of unit tests on them when they do. Developers will also largely be utilizing this capability from packages outside the package where it exists, and I don't feel like making this functionality largely be tested from outside it's own package is a good idea. Maybe I'm not fully understanding your position though.

I've been researching other people's thoughts on testing CMDTs. A good source I have found is this Salesforce developer blog post. In it they essentially suggest creating and storing fake test CMDT records in your org. I'm not a huge fan of that idea if it can be avoided, and I think by adding the right capabilities to our systems, it can be avoided.

Perhaps we just test more than you think is necessary. I know test breadth / depth is a large graph upon which different people often fall in very different quadrants.

@torynet
Copy link
Author

torynet commented Jan 17, 2019

I also just think it's important to test that these units are doing what they are intended to do at this level, so that when they are changed, we can be confident that they are still doing that. I don't like the idea of creating test CMDT records in this package for capabilities that are not used directly in it, just to test the capability. I don't want to create a selector binding configuration here to test that capability, if there are no selectors defined in this package.

@ImJohnMDaniel
Copy link
Collaborator

Hey @torynet

I guess that's just normally how we do things. We test all units. We don't consider whether or not they might be run by the tests of the layers above them. Do you normally only test from the topmost layer? By that logic is there any need for any tests in any of the underlying libraries like Force DI, fflib, etc?

For the most part, I have not yet tested the various configurations of the App Factory level of code. I know that any level above it (i.e. selector, domain, service, UOW, etc.) will test it anyway. Also, with the lack of current support to generate CMDT records in unit tests, the ROI to test the App Factory framework directly was not there for me. Obviously, the ApexMocks, Apex Common, and Force-DI frameworks have some test coverage (albeit, Force-DI does not have exhaustive coverage -- Mainly, again, because of the lack of current support for generating CMDT records in unit tests). Between all of that code and the tests of the actual application directly, the App Factory level will get good coverage.

The reason for creating test config data (which is really a more accurate way of describing it than my previous use of the word "mock"), is that it allows you to test possible configurations rather than just testing the actual configuration you have created. When you create a configurable system, IMHO it's important to test the capability of that system, rather than just individual configurations you may have made that utilize it.

I completely agree with you in principal on this point.

Again, the main issue that I had was not wanting to create real CMDT records only for use by unit tests. IMO, that would be similar to creating logic that says "If Test.isRunning then doA() else doB()" which I am not a big fan of.

I also believe that my issue is a temporary one. I suspect (although have confirmation at this time) that Salesforce will eventually bring out the ability to mock CMDT records in unit tests. When that happens, shoring up the unit test coverage in Force-Di and this project would be more feasible.

Inevitably I will eventually have developers making changes to these classes, and I want the benefits of unit tests on them when they do. Developers will also largely be utilizing this capability from packages outside the package where it exists, and I don't feel like making this functionality largely be tested from outside it's own package is a good idea. Maybe I'm not fully understanding your position though.

TBH, I might be getting lost on what you mean with this paragraph. I believe it would help me to understand the topography and dependency links of the packages you have.

Perhaps we just test more than you think is necessary. I know test breadth / depth is a large graph upon which different people often fall in very different quadrants.

Again, I believe we are basic agreement. I would like to have more test coverage over this codebase (and in Force-DI). In this codebase, it was a discussion that @stohn777 and I had back in the day. We definitely want to circle back and shore up this test coverage. I would welcome any contributions that your team would be able to make to that effort as well.

I also just think it's important to test that these units are doing what they are intended to do at this level, so that when they are changed, we can be confident that they are still doing that. I don't like the idea of creating test CMDT records in this package for capabilities that are not used directly in it, just to test the capability. I don't want to create a selector binding configuration here to test that capability, if there are no selectors defined in this package.

"I also just think it's important to test that these units are doing what they are intended to do at this level, so that when they are changed, we can be confident that they are still doing that. " -- I could not agree more.

Love the discussion! Cheers!

@torynet
Copy link
Author

torynet commented Jan 17, 2019

Just to aid in our understanding each other. Here is our basic package structure. We have put each of the underlying libraries into their own package. We've placed the Application factory in our "Base" package which is I believe equivalent to your "Core" package.

package structure

@stohn777
Copy link
Collaborator

Hi @torynet,

I agree with you and don't like the idea of creating persistent test CMDTs in my orgs either.

Currently tests cannot create CMDT records, but my understanding is that Salesforce will be providing that functionality soon. In the past as a workaround, I've wrapped all usage of my CMDTs in a facade data class, so that test data can be provided by the test as an instance of the facade.

I hope that concise tidbit makes sense, but if not, I'll be happy to detail it further.

Regards.

@torynet
Copy link
Author

torynet commented Jan 17, 2019

@stohn777 That is essentially what I did for the SObjectTypeBinding capability. We ultimately were turning the CMDT configuration records into a Map<String,<Map<SObjectType,SObjectType>>. So, I just made that ultimate data structure available to the tests rather than trying to do something with the CMDT records themselves. I was proposing doing the same thing for the rest of the Application Factory parts.

I believe I will go ahead and implement that solution for us when time permits. It's relatively easy and I don't want to have to wait for Salesforce. Once it's done, I'll submit a PR and let John decide if it's valuable or not. Eventually perhaps Salesforce will give us a better solution, but for now this works.

@torynet
Copy link
Author

torynet commented Jan 17, 2019

I didn't create an actual wrapper class. I just translated the data into a structure of Maps.

@ImJohnMDaniel
Copy link
Collaborator

@torynet -- Apologies for the delay in responding. I certainly understand your position. (Thanks for the dependency diagram. It helped). I am interested in seeing the PR that you mentioned with the structure of Maps. It should definitely help with the discussion. I look forward to it. Cheers!

@stohn777
Copy link
Collaborator

stohn777 commented Mar 2, 2020

@torynet -- Circling back to this issue. You mentioned translating CMDT usage into a mapping architecture. Is that something you're willing to share with the project, and if not, I'd still enjoy seeing your work, just for my edification. Thanks regardless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants