[apex] New rule ApexUnitTestClassShouldHaveRunAs #4149

[tprouvot]

Describe the PR

According to Salesforce best practices, when executing test method we should not rely on the user's running context.
The use of System.runAs is recommended to execute the test with a newly created user so that the test does not rely on environment data and user's configuration

Related issues

• Fixes [apex] New rule: ApexUnitTestClassShouldHaveRunAs #4149

Ready?

• Added unit tests for fixed bug/feature
• Passing all unit tests
• Complete build ./mvnw clean verify passes (checked automatically by github actions)
• Added (in-code) documentation (if needed)

[tprouvot]

Hi @adangel !
I may have missed some steps, it's the first time I create a new rule.
Feel free to reach me if you want any updates on the PR.

[adangel]

Thanks for your PR!

Please have a look at my comments.

Currently the rule basically only checks, whether in the test method at least once System.runAs has been used. Is this sufficient? It's a very simple check... (but ApexUnitTestClassShouldHaveAsserts does only check for at least one assert as well).

Maybe this simple check is enough for the beginning, maybe not - I don't know. That would add up to false-negatives - e.g. Statements that are outside of a RunAs block, but should actually have been inside a RunAs block.

And it will really flag all tests, that don't use System.runAs - maybe there are cases where you don't need a user, since you are testing just some plain logic. That would add up to false-positives.

I'm not an Apex developer, so I can't tell.

The documentation on salesforce also doesn't have a real example, because you won't test a System.debug call as they do in a RunAs... and there are no asserts in the examples as well...

[tprouvot]

Maybe this simple check is enough for the beginning, maybe not - I don't know. That would add up to false-negatives - e.g. Statements that are outside of a RunAs block, but should actu

Hi @adangel thanks for your review !
As you said this rule is pretty simple and a kind of copy of ApexUnitTestClassShouldHaveAsserts.

As you mentioned it, it may exist some cases where we don't need a user to run the test but most of the time we do.
We could say that we don't need runAs if we do not perform CRUD operation in the test method but it is hard to know when we call external methods which is frequently used to create test data..

I think this version is a good start and we will see if we need to extend the way it works.

[pmd-test]

	1 Message
📖	Compared to master: This changeset changes 0 violations, introduces 141 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report

Generated by 🚫 Danger

[adangel]

LGTM, thanks! 🎉

I think this version is a good start and we will see if we need to extend the way it works.

Yes, let's see how the feedback is.

I'll merge it later today.