Write environment aware code
In Rails world, there’s lots of time, we see code like this:
if Rails.env.production?
# Do some dangerous stuffs
else
# Do less dangerous
endMostly, when app interact with 3rd party services. Like, send to different channel in slack, append some test info in message, or just log instead of call out. It works, except sometimes, error happens on production only. And all the developers convince that, “It works on my local”, QAs convince “It works normally on staging. I just tested yesterday.”. You’re all right, but problem is, the code you test isn’t the code running on production. Or worse, you don’t have any unit test in you 3hours/build testing on CI cover # Do some dangerous stuffs (jk, our test in TINYpulse only takes 1hour/build).
The problem is this pattern is untestable and ugly. Any changes which make green on CI, passed all test stages may break production just because of this simple check.
Deeper, it’s an architecture issue, as we’re abusing Rails.env.production?. The application on any stages should run with production mode. There’s nothing Rails.env.staging? nor Rails.env.pre_production?. The app should run on production mode with different configuration. on production stage, we use external call config, on pre-production stage, we use a log call config. With this concept, we could use any config on any stages, and have test on all logic for each config. Sound good?
Hands on, this one of many cases I’m working on to remove Rails.env.production? from TINYpulse apps.
Slack message
The fun part is, half of it is for checking environment and correct behaves on “non-production“ stages, not the main function, build a general template and send slack message. In those case, we append Rails.env to message and write to a specific channel for test; send normally on production; otherwise, do nothing.
Let’s break it.
1. Write the interface
Luckily, this class is written as a command already, input channel and optional emoji. Notify plain message or formatted message with that config.
Ignore the name space, I’m moving it out. But see, it’s purely the SlackNotifer above ’s interface or abstract, you may name, but nothing more.
Why we need this? 1st, it’s used for do nothing case, even we dont have this case, we should create this, like a contract interface, and a mockup when running test, unit tests, integrate test. All the cases we don’t test it (slack notify), using this mock save us a lots of time and preparation.
2. Write the functional code
Then the main course — the functional code
When needn’t care about stages, the code quite simple. Just a note, inherit it from the mock/interface. Implement the body as you wish.
Here, create a client, then wrap the notify calling.
3. Protect the sensitive
The final class, the protected one, it also do stuff, but less danger, for internal/dev only.
It do exactly the same as Slack::Client by calling parent’s behaves, but with censored, protected inputs.
All 3 classes above, we could/should easily write unit test for them. Just focus to how’s it functional, not who or where to use them.
4. Create a switch
Finally, how do each stage knows which class it use:
Rails has Rails.configuration.x for cases like this. You may have
It’s clearly better than a messed of conditions in function class. Application create exactly object with single function as we configured.
Actually, I think it’s SRP — Single Responsibility Principle. Extract condition check from functional, separate null, protected, normal functional. And finally, replace logic by config.
Trade off
It doesn’t mean we have the golden key. Everything have trade off.
First, the config/environments/*.rb files will be longer. But consider, if it’s too long, means there’s too many different between stages. Testing between too different stages like that is not a good way, QAs will quickly turn into the problem bug verified on staging but still happens on production . Grouping, use feature flag/management is also a good point here.
Performance is second issue I think about. it isn’t slow like db query, but keep calling constantize isn’t a good practice. We could cache in to class ininitializers or 1st call. But testing will be a bit more complex. It’s all your call.
tl; dr
Sorry, I cannot shorten this topic in one paragraph. Just keep in mind that Rails.env.production? is evil, avoid it. And there’s many ways to avoid it.
