Is it necessary to SeeAllData in this case? Thoughts on Best Practice?

Question

There's a lot of talk about SeeAllData being something to always avoid for best practice. My understanding is, if you're using it to test, for example, a Contact Trigger on a specific Contact, this is terrible practice, because you can't/shouldn't rely on specific data points like that.

In an example scenario that I'm going through right now, an email is sent in my code, and the email address is set with a Custom Setting field.

My test is failing, because it doesn't have visibility into the custom settings (my assumption*). In this case, would I not need SeeAllData? Is this bad practice? If so, what is the proper way to operate this test class instead? Is the reliance on the custom setting in itself the bad practice?

Looking for advice, because the way I see it, there are certain reasons why you might need SeeAllData which exploit the fact that you're not using best practice, but I don't think using it in itself is bad (I mean, it exists to use in the first place for a reason, right?) Thoughts?

I figured using custom settings for the email address is great because otherwise, I'm hard-coding and email address (one that should be something else in Test so that it doesn't spam when we're testing). But now I'm wondering if it's bad practice since it technically is relying on the database...

*my test fails saying the email address is null and can't send the message. I use SeeAllData, and it sends.

Bonus: While on the topic, what are valid uses of SeeAllData? Every resource I find explains why it's terrible to use, but I think it depends on why you need it

Answer 1

Brian & David have covered well the core of your question, but let's also take a look at this:

Is the reliance on the custom setting in itself the bad practice?

If a) this is new code with a new custom setting and b) it is not a hierarchy custom setting where you are actively using the hierarchical feature to provide different email address values by user or profile, then yes, using the custom setting is not current best practice. You should use a custom metadata type instead, or maybe even a custom label. Both of which are metadata and so exist in the testing context without needing to insert them. You can use custom metadata types in a fashion similar to hierarchical custom settings, but it's a lot more work on the dev end.

This also sheds some light on your more existential question of whether to use seeAllData = true. Custom Settings are a bit weird because they're mostly used in a metadata fashion, and they are included with the metadata from Production when creating a refreshing a Sandbox. But they can't be migrated as metadata except in the Sandbox creation scenario, and are treated as data for tests.

Answer 2

The only known valid use of SeeAllData is for things that explicitly tell you in the documentation that you need SeeAllData, which I believe is only the ConnectApi library. I believe there was also a bug regarding unique indexes, and for that case, you might also need SeeAllData.

For every other situation, you should not use it. In the example of your custom setting, you can insert custom settings in a unit test. They will not persist beyond the test, obviously, and will not conflict with live custom setting data.

The reason why you don't want to use SeeAllData is because of migration. If you have a record in a sandbox, and you try to migrate code to production, your test might fail because that same record doesn't exist. Or maybe a user decides to delete your important record, and breaks all deployments until you fix the problem.

This is doubly true if you use hard-coded ID values. If you have a Very Important Record, and you specify it by ID, and someone decides to delete this record and purge the recycle bin, you are now forced to fix your unit tests and/or code, because you can't ever recover the record. This is why you're also told to not depend on hard-coded ID values whenever possible.

Answer 3

You don't need SeeAllData. Your test class can and should construct and insert Custom Settings instances, something you'll often do in an @testSetup method. They're constructed and inserted just like normal sObjects.

You can populate the SetupOwnerId field appropriately if it's a Hierarchy Custom Setting. In most cases unit tests can ignore hierarchy, however.

The number of acceptable uses of SeeAllData are few and are edge cases. One that came up recently on SFSE was use of the ConnectAPI (Chatter), which requires it to call ConnectAPI methods in a unit test. However, using mocking and dependency injection to avoid calling that API in test context at all, while still verifying behavior, is in my opinion a better route.