RHSTOR-7461Upgrade Yup to version 1.6.1 #2139
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: NIKHITHAVADDEMPUDI The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@NIKHITHAVADDEMPUDI: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| @@ -49,10 +49,10 @@ describe('useYupValidationResolver tests', () => { | |||
| messages: { | |||
| ['Required']: { | |||
| field: 'name', | |||
| type: 'required', | |||
| type: 'optionality', | |||
There was a problem hiding this comment.
@NIKHITHAVADDEMPUDI Why is this change required?
There was a problem hiding this comment.
Without the above change the code is failing yarn test and showing the below error
useYupValidationResolver tests › should return an obj with values property with empty obj and errors property with valid object when fails validation
expect(received).toMatchObject(expected)
- Expected - 2
+ Received + 2
@@ -3,13 +3,13 @@
"name": Object {
"message": "Required",
"messages": Object {
"Required": Object {
"field": "name",
- "type": "required",
+ "type": "optionality",
},
},
- "type": "required",
+ "type": "optionality",
},
},
"values": Object {},
}
62 | });
63 |
> 64 | expect(actual).toMatchObject(expected);
| ^
65 | expect(typeof result.current).toBe('function');
66 | expect(validateSpy).toHaveBeenCalledTimes(1);
67 | });
at Object.toMatchObject (packages/shared/src/yup-validation-resolver/useYupValidationResolver.spec.ts:64:20)
There was a problem hiding this comment.
@NIKHITHAVADDEMPUDI we must understand the rationale behind, just because the test pass does not mean it's correct: a misleading test is worse than no test: how is that it's not required anymore?
There was a problem hiding this comment.
I see that the field is still required (nullable: false, optional: false), so it seems we're good here:
name: StringSchema {
type: 'string',
_typeCheck: [Function: check],
_whitelist: ReferenceSet(0) [Set] {},
_blacklist: ReferenceSet(0) [Set] {},
internalTests: {
typeError: [Function],
nullable: [Function],
optionality: [Function]
},
exclusiveTests: { required: false },
deps: [],
conditions: [],
tests: [ [Function] ],
transforms: [ [Function (anonymous)] ],
spec: {
strip: false,
strict: false,
abortEarly: true,
recursive: true,
disableStackTrace: false,
nullable: false,
optional: false,
coerce: true
},
_mutate: undefined
},
| return { obcFormSchema, fieldRequirements }; | ||
| return { | ||
| obcFormSchema: | ||
| obcFormSchema as unknown as UseObcBaseSchema['obcFormSchema'], |
There was a problem hiding this comment.
do we need to convert it to unkown first before casting to correct type ??
There was a problem hiding this comment.
Directly converting to the correct type is showing the below error
There was a problem hiding this comment.
it's a hack... doing this defeats the purpose of using types...
There was a problem hiding this comment.
see what's the issue and fix that... same for other places...
There was a problem hiding this comment.
try Yup.InferType and check if it works...
| @@ -44,7 +44,7 @@ export const providerSchema = (shouldValidateSecret: boolean) => | |||
| }), | |||
| 'pvc-name': Yup.object().when('provider-name', { | |||
| is: StoreProviders.FILESYSTEM, | |||
| then: (schema: Yup.SchemaOf<PersistentVolumeClaimKind>) => | |||
| then: (schema: Yup.ObjectSchema<PersistentVolumeClaimKind>) => | |||
There was a problem hiding this comment.
what's the diff bw SchemaOf and ObjectSchema ??
There was a problem hiding this comment.
I got some info from internet sources: Yup.SchemaOf is more generic, can be used for arrays, objects etc, whereas Yup.ObjectSchema is used only for validating objects.
https://issues.redhat.com/browse/RHSTOR-7461
Upgrade yup to version 1.6.1 in both package.json files
Upgradation of yup is leading to errors as in the attached file yup_errors.txt
yup_errors.txt
Yarn test is leading to the errors as in the attached file yarn_test_yup_errors.txt
yarn_test_yup_erros.txt
hence updated the file packages/shared/src/yup-validation-resolver/useYupValidationResolver.spec.ts to resove the errors on yarn test
and modified the files packages/odf/components/mcg/useObcFormSchema.ts
packages/odf/components/s3-browser/create-bucket/useS3BucketFormValidation.ts
packages/odf/constants/providerSchema.ts
to resolve type errors