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

Feature: required() if process.env.NODE_ENV != 'test' #23

Closed
gunar opened this issue Oct 23, 2018 · 9 comments
Closed

Feature: required() if process.env.NODE_ENV != 'test' #23

gunar opened this issue Oct 23, 2018 · 9 comments

Comments

@gunar
Copy link

gunar commented Oct 23, 2018

Would you be willing to implement it? I can send a PR too if you need me to

@evanshortiss
Copy link
Owner

evanshortiss commented Oct 23, 2018

@gunar it sounds like you'd like to be able to have the required() function behave differently based on a specific condition - in your case that condition is the value NODE_ENV?

Would you mind providing a code example of how you would like to see this feature work?

@gunar
Copy link
Author

gunar commented Oct 25, 2018

@evanshortiss My unit tests require files that require the config.js file, which forces me to have the env vars set in order for tests to pass.

Unit tests don't depend on any environment variable (e.g. AUTH_TOKEN), but I still want to use env-var ❤️


Code example

// src/config.js
import * as env from 'env-var'

export const AUTH_TOKEN = env.get('AUTH_TOKEN').required().asString()
// src/businessLogic.js
import * from src/config

// ... business logic code ...
// test/businessLogic.spec.js

import * from src/businessLogic

// ... tests ...

@evanshortiss
Copy link
Owner

I understand now. I don't think NODE_ENV=test is standard so I would be very cautious adding it to env-var since people will not expect that behaviour, right?

I have some thoughts:

  1. Use stubs or mocks in your tests. This is probably the best choice in general.
  2. Modify the config.js to change behaviour in test mode.
  3. Add a new feature to env-var.
  4. Use dependency injection - I list this since it's an option, but might need you to restructure modules

Option 1

Using mocks in your unit tests will allow you to stub/mock out config.js entirely. This is a good approach since you can adjust it for each test case if necessary. I'm not sure what text framework you use, but Jest supports class mocks and has a host of other mocking features. If not using jest, then maybe use proxyquire with mocha or another test runner?

Option 2

You could use fake values if in test mode:

import * as env from 'env-var'

const isTest = env.get('NODE_ENV').asString() === 'test'

const AUTH_TOKEN = isTest ? 'FAKE_VALUE' : env.get('AUTH_TOKEN').required().asString()

Option 3

Add a feature like this in env-var where required accepts an optional parameter function to customise the behaviour.

import * as env from 'env-var'

const isRequired = () => env.get('NODE_ENV').asString() !== 'test'

// required only throw if NODE_ENV is != "test"
export const AUTH_TOKEN = env.get('AUTH_TOKEN').required(isRequired).asString()

@evanshortiss
Copy link
Owner

option 4

// src/businessLogic

// We do not require config.js here, instead we have it passed by the callee
export default function (config) {
  // use config.AUTH_TOKEN
}

Then in the test:

// test/businessLogic.spec.js

import businessLogic from src/businessLogic

const bl = bl({ AUTH_TOKEN: 'fake token for testing' })

Or in real code:

import businessLogic from src/businessLogic
import * from src/config as config

const bl = bl(config)

@evanshortiss
Copy link
Owner

@gunar let me know your thoughts

@evanshortiss
Copy link
Owner

@gunar closing this, but if you have any further input feel free to reopen.

@evanshortiss
Copy link
Owner

@gunar, check out the new "required()" function behavior. It can support this now.

@gunar
Copy link
Author

gunar commented Mar 14, 2019

Thanks @evanshortiss !

@gunar
Copy link
Author

gunar commented Mar 14, 2019

const SECRET = env.get('SECRET', 'bad-secret').required(NODE_ENV === 'production').asString()

Very smart.

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

No branches or pull requests

2 participants