-
Notifications
You must be signed in to change notification settings - Fork 2
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
test: Cover error handling cases in API tests (1/2) #2466
Conversation
Removed vultr server and associated DNS entries |
@@ -7,7 +7,6 @@ import { Flow } from "../../../types"; | |||
const copyPortalAsFlow = async (flowId: string, portalNodeId: string) => { | |||
// fetch the parent flow data | |||
const flow = await getFlowData(flowId); | |||
if (!flow) throw Error("Unknown flowId"); |
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 was redundant and unreachable, this exception is already thrown inside getFlowData()
@@ -44,7 +45,7 @@ export async function sendToEmail( | |||
// Prepare email template | |||
const config: EmailSubmissionNotifyConfig = { | |||
personalisation: { | |||
serviceName: flowName || "PlanX", |
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 was also unreachable - all flows must have names.
@@ -54,12 +55,7 @@ export async function sendToEmail( | |||
|
|||
// Send the email | |||
const response = await sendEmail("submit", sendToEmail, config); | |||
if (response?.message !== "Success") { |
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.
Again, this code was unreachable - message
is only set to "Success"
and nothing else.
}); | ||
await expect(sendEmail("save", TEST_EMAIL, mockConfig)).rejects.toThrow(); | ||
}); | ||
}); |
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.
These are super nice & clear 🙌
* test: More coverage updates * fix: Linting * fix: Copy paste errors
7428833
to
e347881
Compare
What does this PR do?
API test coverage change in #2466 & #2482