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

tests fails if I there is global configuration #211

Closed
sarbbottam opened this issue Apr 25, 2016 · 3 comments
Closed

tests fails if I there is global configuration #211

sarbbottam opened this issue Apr 25, 2016 · 3 comments

Comments

@sarbbottam
Copy link
Contributor

吽 cat ~/.czrc
{ "path": "cz-conventional-changelog" }
  10 passing (54s)
  12 failing

  1) adapter resolves adapter paths:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  2) adapter gets adapter prompter functions:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  3) commit should commit simple messages:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  4) commit should commit message with quotes:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  5) commit should commit multiline messages:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  6) commit should allow to override options passed to gulp-git:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  7) init installs an adapter with --save-dev:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  8) init installs an adapter with --save:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  9) init errors on previously installed adapter:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  10) init succeeds if force is true:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  11) init installs an adapter without --save-exact:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)
  12) init installs an adapter with --save-exact:
     Error: the string "A previous adapter is already configured. Use --force to override" was thrown, throw an Error :)

If I remove or rename ~/.czrc, all the tests passes.

@jimthedev
Copy link
Member

This seems like a bug with our tests not being sandboxed properly. They are leaking. This is one of the tricky things with our current test suite. Since the loader being run during the tests actually hits disk and walks the directory tree we need a way of sandboxing the upmost directory during tests only.

This is to say, we need some way of saying to the loader: hey you're testing so you should never search higher than ~/path/to/cz-cli/tests

Currently the loader sees your global config even during tests. We should have some way of ignoring it. This currently does not exist and thus this is somewhere between a feature and a bugfix.

I think at one point we had something like this but it was removed to simplify the loader. Since a global config was not a use case at the time, we went for simplicity over added features. I am totally fine with this coming back in so tests can be run properly. Good catch.

@sarbbottam
Copy link
Contributor Author

This is to say, we need some way of saying to the loader: hey you're testing so you should never search higher than ~/path/to/cz-cli/tests

I guess, if we set process.env.USERPROFILE, process.env.HOMEPATH, process.env.HOME to ~/path/to/cz-cli/tests during tests, it should be ok, isn't it?

var directoryArr = [process.env.USERPROFILE, process.env.HOMEPATH, process.env.HOME];

(Haven't tried it, just wild guess)

@sarbbottam
Copy link
Contributor Author

sarbbottam commented Apr 28, 2016

I guess, if we set process.env.USERPROFILE, process.env.HOMEPATH, process.env.HOME to ~/path/to/cz-cli/tests during tests, it should be ok, isn't it?

     "report-coverage": "nyc report --reporter=lcov | codecov",
     "semantic-release": "semantic-release pre && npm publish && semantic-release post",
     "start": "npm run test:watch",
-    "test": "nyc --require babel-core/register _mocha -- test/tests/index.js",
+    "test": "USERPROFILE=test HOMEPATH=test HOME=test nyc --require babel-core/register _mocha -- test/tests/index.js",
     "test:watch": "nodemon -q --ignore test/.tmp/ --ignore test/artifacts/ --ignore coverage/ --exec \"npm run test\" --",
     "test:windows": "node ./test/tools/trigger-appveyor-tests.js"

It works, but, babel fails, as I have overridden the HOME and its looking for .babel.json at .test/

fs.js:634
  return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
                 ^

Error: ENOENT: no such file or directory, open 'test/.babel.json'
    at Error (native)
    at Object.fs.openSync (fs.js:634:18)
    at Object.fs.writeFileSync (fs.js:1327:33)
    at process.save (/Users/sabandyo/GitHub/cz-cli/node_modules/babel-core/node_modules/babel-register/lib/cache.js:48:19)
    at emitOne (events.js:96:13)
    at process.emit (events.js:188:7)
    at processEmit [as emit] (/Users/sabandyo/GitHub/cz-cli/node_modules/nyc/node_modules/signal-exit/index.js:140:35)
    at process.exit (internal/process.js:79:15)
    at done (/Users/sabandyo/GitHub/cz-cli/node_modules/mocha/bin/_mocha:416:32)
    at afterWrite (_stream_writable.js:360:3)
    at _combinedTickCallback (internal/process/next_tick.js:80:20)
    at process._tickCallback (internal/process/next_tick.js:98:9)

I guess code change is required.

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

Successfully merging a pull request may close this issue.

2 participants