test(core): catch type issues when configuring from external json #14101
Conversation
ashwinkumar6
requested review from
sktimalsina,
cshfang,
HuiSF,
pranavosu and
a team
as code owners
| // Ensures that all optional properties in mockAmplifyOutputs are defined | ||
| const _amplifyOutputs: DeepRequired<AmplifyOutputs> = | ||
| recreateObject(mockAmplifyOutputs); |
There was a problem hiding this comment.
Didn't quite get this - are we validating all possible fields are contained in the amplify_outputs.json file?
There was a problem hiding this comment.
Yep exactly, when a new optional key is added in AmplifyOutputs we want to make sure the mockAmplifyOutputs used here is also updated to include this field. With this approach if amplify_outputs isn't updated this line would crib
There was a problem hiding this comment.
Got it. amplify_outputs.json will be updated manually? Is there any checks too that the manual update conforms to CLI JSON schema?
There was a problem hiding this comment.
+1 we need to figure out a way to make sure our outputs.json conforms to CLI schema.Might be out of scope for this PR.
| "require_symbols": true, | ||
| "min_length": 6 | ||
| }, | ||
| "groups": [ |
There was a problem hiding this comment.
this was previously
{
"ADMIN": { "precedence": 0 },
"USER": { "precedence": 1 }
},
{
"USER": { "precedence": 0 }
},
{
"TEST": { "precedence": 2 },
"USER": { "precedence": 1 },
"ADMIN": { "precedence": 0 }
}
since backend can only generate one userGroup as one item in the array atm, updated to this format.
|
One Major comment I have for this approach is this will only catch us from missing addition of optional properties in amplify_outputs. It does not catch the core issue on if the value we provide for these properties actually causes an issue or not. |
Description of changes
authentication_flow_typetype from string literal union to stringamplify_outputs.jsonhas all combinations defined, else throw type error (CI failure)Amplify.configure(CI failure)Description of how you validated changes
linked ^
Checklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.