Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add unit tests for error states in critical workflows #1118
feat: add unit tests for error states in critical workflows #1118
Changes from all commits
f31cd08
7c68bb2
c587fb3
050b791
d755b58
61f23e6
e10507b
f8d1b4e
8421eaa
906227a
7039d30
982c477
7707135
5df18c0
7d974b2
995cec9
6d45220
bfaf022
a19dd63
76c6f15
4203098
03f4079
8e3077c
de0aa7e
2a30995
417f586
2b26014
2a17af1
f94cf21
1006182
6dbd3fa
f39cae6
60d50a2
74b94ea
18226dc
08d3aab
aefeac7
b986412
ffad95d
0b6b279
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
We could also test for the error messages output when invalid templates are provided such as
{{test}
,{{test 123}}
{{}}
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.
Do you want to add these in? i think the reason why I gave these examples is that they each output a different error message, but if the error messages are generated on the backend and not the frontend, then we can address this in backend tests
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 looked into them initially, but realised that I would need to add some extra cases to the API endpoint mocks to handle them, so I put that on hold 😅
Now that you mention it, having these in the backend tests sounds like a good idea as well! If so, I guess we can postpone adding frontend tests for those cases to later?
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.
Possibly the setup and teardown in these tests could be extracted to jest before/after hooks
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 it's a bit difficult to do so, since most tests have slightly different setup/teardown phases. For example, some tests mock
console.error
and some use protected email campaigns.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.
Alright, as for mocking the
console.error
, if you want to do this globally, you can feed it in a setup file in yourjest.config.ts
, this is something the backend tests currently implement to silenceconsole.log
messagesThere 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.
Good point. I considered doing so, but decided against mocking
console.error
globally. When fixing test failures, I found thatconsole.error
statements provided useful information.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.
Sure, the backend tests currently implement that only for
console.log
to not overcrowd the travis log output