-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Decouple process.env #1688
Decouple process.env #1688
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.
LGTM, just needs changes to the support code for feature tests I think.
Also not sure if there's anything specific we need to do for parallel runtime?
@@ -41,6 +41,7 @@ export interface IConfiguration { | |||
export interface INewConfigurationBuilderOptions { | |||
argv: string[] | |||
cwd: string | |||
env: Record<string, string | undefined> |
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.
Could type as ProcessEnv
from @types/node
?
Nice! |
@aslakhellesoy is this still a live problem for you? I notice it has a lot of merge conflicts now. |
@mattwynne I think these changes were effectively added under #1849, looking at the state of the files now seems like the same outcome. |
Great, thanks for the update David. I'll close the PR. |
This is necessary in order to make tests behave independently of the value of environment variables such as
CUCUMBER_PUBLISH_TOKEN
.