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

Add restore option #99

Merged
merged 8 commits into from
Jun 12, 2019
Merged

Add restore option #99

merged 8 commits into from
Jun 12, 2019

Conversation

yinzara
Copy link
Contributor

@yinzara yinzara commented Jun 4, 2019

Some tests may want to use the natural system environment but then restore it completely after the test (so that any environment variables added during the test get removed). Adding a 'restore' option allows for this.

Some tests may want to use the natural system environment but then restore it completely after the test (so that any environment variables added during the test get removed).  Adding a 'restore' option allows for this.
@bahmutov
Copy link
Owner

bahmutov commented Jun 5, 2019

I like this option, but would like to see a test added

this feature adds a restore option that if passed will restore the process.env environment when the restore() function is called. This varies from the current "clear" functionality as it does not clear the current environment when mockedEnv is called.
@yinzara
Copy link
Contributor Author

yinzara commented Jun 6, 2019

Tests added. Do you want me to rebase the branch as there are commits that don't pass lint.

@yinzara
Copy link
Contributor Author

yinzara commented Jun 9, 2019

I also took the liberty of adding TypeScript definitions.

)
})

it('has default env, BANG, FOO and BAR', () => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I am not sure this is the best test for this. We want to check "restore" right? So we probably better do

const restore = mockedEnv({...}, {restore: true})
process.env.BANG = 'bash'
restore()
la(process.env.BANG === undefined, 'did not restore process.env')

what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That test only verifies that the environment variable that was present before mocking the environment was available during the mock when "restore" is given.

The later test "pass 'restore' in options" actually tests the case you're referring to where it changes the variable via process.env after mocking then verifies it restores it after.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, merging your PR now

@bahmutov bahmutov merged commit 90d3234 into bahmutov:master Jun 12, 2019
@bahmutov
Copy link
Owner

🎉 This PR is included in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants