-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Change testing NODE_ENV to "lerna-test" (Fixes #406) #440
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -18,7 +18,7 @@ class ProgressBarController { | |||
} | |||
|
|||
// Don't do any of this while testing | |||
if (process.env.NODE_ENV === "test") { | |||
if (process.env.NODE_ENV === "lerna-test") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the progress bar does look a bit wonky in CI logs. wonder if there's a better flag here... maybe env.CONTINUOUS_INTEGRATION
or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, wonky whenever we're not running interactively.
Maybe a better check here would be whether we're connected to a tty?
if (process.stdout.isTTY) {
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's irrelevant to this PR anyway, just a quirk i noticed after i unset NODE_ENV
on circle. probably best to add a FIXME
and sort this out later, since it's not that important
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for catching this! Is there any way you can release this sooner rather than later? It's a blocker for people relying on tests to fail a build. Right now, a build can pass even if the tests fail because the exit codes are not propagated. I'm experiencing this right now with CircleCI. I considered pulling in the master branch in, but in order to use it, you'll need to run |
This ought not be a blocker with Circle. Just do dependencies:
override:
# Circle sets NODE_ENV to test by default, which messes with both Lerna and
# babel-plugin-dev-expression.
- unset NODE_ENV
- npm i |
This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
fixes #406
Previously, when running Lerna with
NODE_ENV
set to "test", it would output nothing to stdout and always exit with a code of0
. Now the output and exit code will not be suppressed.