Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Fixed issue where if local.js exists then grunt test will run on that… #824

Merged

Conversation

lirantal
Copy link
Member

… environment config and possibly delete collections

We only extend the config object with the local.js custom/local environment if we are on production or development environment. If test environment is used we don't merge it with local.js to avoid running test suites on a prod/dev environment (which delete records and make modifications)

@lirantal lirantal self-assigned this Aug 19, 2015
@lirantal lirantal added this to the 0.4.x milestone Aug 19, 2015
@codydaig
Copy link
Member

@lirantal In this case do we still want/need the double variable declaration?

// Don't need the envConf var anymore. 
// var envConf = _.merge(defaultConfig, environmentConfig);
var config = _.merge(defaultConfig, environmentConfig);

if (process.env.NODE_ENV !== 'test') {
  config = _.merge(conf, (fs.existsSync(path.join(process.cwd(), 'config/env/local.js')) && require(path.join(process.cwd(), 'config/env/local.js'))) || {});  
  }

@lirantal lirantal force-pushed the bugfix/localjs-env-params-used-for-tests branch from 68f3d58 to acdf3e7 Compare August 19, 2015 18:12
@lirantal
Copy link
Member Author

Thanks @codydaig , I updated it.

@codydaig
Copy link
Member

@lirantal You also have to change envConf to config on line 168. Sorry, I didn't make that very clear.

… environment config and possibly delete collections

We only extend the config object with the local.js custom/local environment if we are on production or development environment. If test environment is used we don't merge it with local.js to avoid running test suites on a prod/dev environment (which delete records and make modifications)
@lirantal
Copy link
Member Author

Sure, my bad. Long day this is :)

Just proves how much more tests we really need on the framework itself to make sure we're not messing things up.

@lirantal lirantal force-pushed the bugfix/localjs-env-params-used-for-tests branch from acdf3e7 to d188326 Compare August 19, 2015 19:06
@codydaig
Copy link
Member

@lirantal LGTM! Thanks! :-D

@lirantal
Copy link
Member Author

Thank you

lirantal added a commit that referenced this pull request Aug 19, 2015
…for-tests

Fixed issue where if local.js exists then grunt test will run on that…
@lirantal lirantal merged commit 2e8d659 into meanjs:master Aug 19, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants