-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add tests for apiErrors and standardErrors #48
Conversation
@@ -242,6 +242,6 @@ en: | |||
fetchTaskStatus: "There was an error fetching the task status while syncing sandboxes." | |||
fetchTypes: "There was an error fetching sandbox types." | |||
processFieldsJs: | |||
fieldsJsNotReturnArray: "There was an error loading JS file \"{{ path }}\". Expected type \"Array\". Make sure that your function returns an array" | |||
fieldsJsNotFunction: "There was an error loading JS file \"{{ path }}\". Expected type \"Function\". Make sure that your default export is a function." | |||
fieldsJsNotReturnArray: 'There was an error loading JS file "{{ path }}". Expected type "Array". Make sure that your function returns an array' |
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 didn't purposely do this. Maybe prettier did it?
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.
Ohh yep I think prettier prefers not to use escape characters when it can avoid it.
err.statusCode === 403 && | ||
err.error && | ||
err.error.category === 'MISSING_SCOPES' | ||
export function isMissingScopeError(err: GenericError): boolean { |
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 thinking here is that we aren't sure which type of error we're going to get as an arg. The calling system may be passing something in that's more generic and doesn't necessarily have the type of StandardCodeError
.
): never { | ||
const i18nKey = 'errors.api'; | ||
const i18nKey = 'errors.errorTypes.api'; |
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 noticed we were getting missing translation errors here
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.
This all LGTM. Tests look good + I think the generic error pattern makes more sense that what we were doing previously
@@ -242,6 +242,6 @@ en: | |||
fetchTaskStatus: "There was an error fetching the task status while syncing sandboxes." | |||
fetchTypes: "There was an error fetching sandbox types." | |||
processFieldsJs: | |||
fieldsJsNotReturnArray: "There was an error loading JS file \"{{ path }}\". Expected type \"Array\". Make sure that your function returns an array" | |||
fieldsJsNotFunction: "There was an error loading JS file \"{{ path }}\". Expected type \"Function\". Make sure that your default export is a function." | |||
fieldsJsNotReturnArray: 'There was an error loading JS file "{{ path }}". Expected type "Array". Make sure that your function returns an array' |
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.
Ohh yep I think prettier prefers not to use escape characters when it can avoid it.
Description and Context
Adding some basic unit tests to expand our test coverage. This focuses on the error utils.
Screenshots
TODO
Who to Notify