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

Discussion: environment setting #66

Open
DanielGluskin opened this issue Jan 24, 2021 · 3 comments
Open

Discussion: environment setting #66

DanielGluskin opened this issue Jan 24, 2021 · 3 comments

Comments

@DanielGluskin
Copy link
Collaborator

As we arrange some tests with:
process.env.SEND_MAILS = "true";

We need to clean this value on end but we cannot simply set it to “false” as it might actually originally be “true”.

Instead we might restore the environment back to its initial state after each run:

BeforeAll(() => { defaultEnv = process.env …
AfterEach(() => { process.env = defaultEnv ...

On the other hand, let’s assume my local env.SEND_MAILS is set to “false”. When I block all external calls in a test - it will pass on my machine. But if the same test is run on another machine, in which env.SEND_MAILS is set to “true”, it will fail.

Should each test declare a SEND_MAILS value in its setup? in contrast to the previous point.

What should be considered as a good practice here? Maybe tests should be aware whether they test a predefined behaviour, in which we set SEND_MAILS in the beginning. Others should test a concrete deployment behaviour - in which we do not change the environment, and if we do, we use the first practice and restore to default.

Your opinion?

@mikicho
Copy link
Contributor

mikicho commented Feb 3, 2021

I think we don't need to use env_vars for business logic.
Why do we need this env var at all?

@goldbergyoni
Copy link
Contributor

@mikicho It's not related to business logic specifically, it's very common that projects use flags like this (e.g. send mail or anything else) to be able to adjust them in production. As @DanielGluskin alluded, it does bring an interesting challenge of how to clean-up because there is no magical sinon.restore() here.

I would research the price of detecting the defaults when the suite starts and restoring to this after each.

This might end in our package environemtVariableSandbox.restore().

Thoughts?

@DanielGluskin
Copy link
Collaborator Author

DanielGluskin commented Feb 5, 2021

I'm not raising this issue to say whether they should be used or not - currently - we do use them and do not clean up properly.

I also want to point two different senarios - in case we restore, like @goldbergyoni said, we check somehow the "logic" - if MAIL_FALG, then send mail and so on...

This approach is a bit like mocking, we do not hit the real environment setup. If my prod MAIL_FLAG is true, and most of the tests are run with this flag as false, do I really check what I want to check?

I want to be able to check if my production intance is doing what I expect it to do - in this case we cannot use the previous practice mentioned.

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

No branches or pull requests

3 participants