-
Notifications
You must be signed in to change notification settings - Fork 17
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
chore(config): check test environment for feature flags #1665
base: main
Are you sure you want to change the base?
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.
I think I was not clear enough in my comment #1663 (comment). The envvar definitions have to remain in the validators
object -- they are only meaningful if passed through envalid.cleanEnv()
.
Separating the config from the parsed envvars allows something like:
const config = {
allowSendAll: env.isTest || env.ALLOW_SEND_ALL
};
Got you! Can you say more about what meaningful refers to here? Is it that the validators aren't really validating anymore because they're not being passed through? The failing test for #1663 passed after this commit so i jumped the gun and took that as success 😅 |
The validator (e.g. type CleanEnvOutput = {
isProd: boolean;
ALLOW_SEND_ALL: boolean;
}; but the shape of the type MixedConfigType = {
isProd: boolean;
ALLOW_SEND_ALL: ValidatorSpec<boolean>;
}; The tests likely passed because It might be helpful to look at the whole |
Got it! Maybe I'll briefly check whether I can easily convert the Spoke config to TS without breaking anything too 😅 |
I don't think you need to convert to TS to move forward with this. But I think the types are a good way of understanding what is happening? |
yeah i agree! |
648bcc2
to
b0de04a
Compare
Description
This modifies the config file to enable feature flags for running tests.
Motivation and Context
#1663 (comment)
How Has This Been Tested?
Tests are still passing
Screenshots (if appropriate):
Documentation Changes
Checklist: