feat: add unit tests for error states in critical workflows #1118

zwliew · 2021-04-09T03:48:11Z

Problem

#1110 added tests for the happy paths in critical workflows. It would be a good idea to add tests for the error paths as well to cover our bases.

Workflows:

Creating and sending a new email campaign
Creating and sending a new SMS campaign
Creating and sending a new Telegram campaign
Creating and sending a new protected email campaign
Viewing a protected email message
Unsubscribing from a mailing list

Partially resolves issue #1079

Solution

Features:

Added unit tests for various error states that occur during campaign creation

Before & After Screenshots

There are no visual changes.

Tests

These tests have been verified on Travis and Amplify.

Deploy the change on Travis and Amplify.
All the following tests should be run and pass:

SMS
- Saving an invalid template (empty body after sanitization)
- Attempting to validate an invalid credential
- Uploading an invalid CSV file
Telegram
- Saving an invalid template (empty body after sanitization)
- Attempting to validate an invalid credential
- Uploading an invalid CSV file
Email
- Saving an invalid template (empty subject after sanitization)
- Uploading an invalid CSV file
Protected Email
- Saving an invalid template (empty subject after sanitization) (same as email)
- Saving an invalid template (extraneous subject params)
- Saving an invalid template (missing body params)
- Saving an invalid template (extraneous body params)
- Viewing a protected email with an invalid ID
- Viewing a protected email with an invalid password
- Uploading an invalid CSV file
Write unit tests for the following misc non-error cases (not a priority; simply written for practice):
- SMS
  - Displays the necessary elements when rendered
  - Next button is disabled when template is empty
  - Next button is enabled when template is filled
  - Character count text reflects the actual number of characters in the textbox
- Telegram
  - Displays the necessary elements when rendered
  - Next button is disabled when template is empty
  - Next button is enabled when template is filled
- Email
  - Displays the necessary elements when rendered

Deploy Notes

The aforementioned tests have been added and should be run during every deployment on Travis and Amplify. If any test fails, the deployment should not go through.

Specifically, Amplify should not publish the new build if any test fails.

zwliew · 2021-04-19T10:12:59Z

I've rebased the branch on top of frontend-tests-dashboard

miazima

Here are some suggestions on scenarios we could add, can be done in a later pr too

EmailRecipient

test for invalid file/valid file scenario

SMSCredential

test with invalid phone number like 80000000

frontend/src/components/dashboard/create/email/tests/EmailTemplate.test.tsx

miazima · 2021-04-20T02:58:42Z

frontend/src/components/dashboard/create/email/tests/EmailTemplate.test.tsx

+  // Setup
+  jest.spyOn(console, 'error').mockImplementation(() => {
+    // Do nothing. Mock console.error to silence expected errors
+    // due to submitting invalid templates to the API
+  })
+  server.use(...mockApis(false))
+  renderTemplatePage()


Possibly the setup and teardown in these tests could be extracted to jest before/after hooks

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.

Alright, as for mocking the console.error, if you want to do this globally, you can feed it in a setup file in your jest.config.ts, this is something the backend tests currently implement to silence console.log messages

Good point. I considered doing so, but decided against mocking console.error globally. When fixing test failures, I found that console.error statements provided useful information.

Sure, the backend tests currently implement that only for console.log to not overcrowd the travis log output

miazima · 2021-04-20T03:01:47Z

frontend/src/components/dashboard/create/email/tests/EmailTemplate.test.tsx

+  expect(screen.getByRole('button', { name: /next/i })).toBeInTheDocument()
+})
+
+test('displays an error if the subject is empty after sanitization', async () => {


We could also test for the error messages output when invalid templates are provided such as {{test}, {{test 123}} {{}}

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

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?

miazima · 2021-04-20T03:51:44Z

frontend/src/components/dashboard/create/sms/tests/SMSTemplate.test.tsx

+  expect(screen.getByText(/characters/i)).toBeInTheDocument()
+})
+
+test('next button is disabled when template is empty', async () => {


For tests like these which are common across the channels, is there a way we could share and reuse them?

Good point. I could create a common test for these 2 components, but I think maybe a cleaner option would be to extract the common code in SMSTemplate and TelegramTemplate into a separate component, and write a test.

I'll explore both options.

I've attempted to extract the common code to a BodyTemplate component here: #1148

Maybe this can go in first and then we can refactor the tests once the other pr is approved.

Ok, sounds good!

Make sure to set CI=true explicitly as Amplify doesn't set it by default. CI=true will run all the tests exactly once and exit. Also compile translations before running the tests.

Just a simple render test to ensure that the test infrastructure works fine.

This makes more sense as compilation errors are more important than test errors. This also matches the Amplify build configuration.

Also refactor common API endpoints shared with the Email campaign test into a separate array.

Also add assertions for testing the credential dropdown for SMS campaigns.

…paign Previously, we were selecting the SMS channel button, not the email button.

zwliew · 2021-04-26T07:28:54Z

Sorry for the erased commit history -- I've rebased the branch atop of develop. The latest changes start from commit 74b94ea, feat: mock API responses for invalid CSV files

* develop: refactor: use shared function to initialize models (#1172) chore: setup scaffolding for backend tests (#940) 1.23.0 fix(frontend): fix frontend test flakiness (#1162) fix: update successful delivery status only if error does not exist (#1150) chore: upgrade dependencies (#1153) feat: add unit tests for error states in critical workflows (#1118) feat: support whitelisting domains through `agencies` table (#1141) feat: add tests for happy paths in critical workflows (#1110) fix: prevent campaign names from causing dashboard rows to overflow (#1147) fix(email): Fix SendGrid fallback integration (#1026)

* develop: feat: refactor msg template components; add telegram character limit (#1148) refactor: use shared function to initialize models (#1172) chore: setup scaffolding for backend tests (#940) 1.23.0 fix(frontend): fix frontend test flakiness (#1162) fix: update successful delivery status only if error does not exist (#1150) chore: upgrade dependencies (#1153) feat: add unit tests for error states in critical workflows (#1118) feat: support whitelisting domains through `agencies` table (#1141) feat: add tests for happy paths in critical workflows (#1110) fix: prevent campaign names from causing dashboard rows to overflow (#1147)

zwliew changed the base branch from develop to frontend-tests-dashboard April 9, 2021 03:54

zwliew added the frontend Frontend label Apr 9, 2021

zwliew force-pushed the frontend-tests-dashboard branch 2 times, most recently from 44ec28f to 8931c6d Compare April 12, 2021 09:59

zwliew force-pushed the frontend-tests-error-states branch 3 times, most recently from da76289 to 93f1f0b Compare April 13, 2021 04:57

zwliew marked this pull request as ready for review April 13, 2021 05:00

zwliew changed the title ~~feat: add unit tests for campaign creation error states~~ feat: add unit tests for error states in critical workflows Apr 19, 2021

zwliew changed the base branch from frontend-tests-dashboard to develop April 19, 2021 09:12

zwliew force-pushed the frontend-tests-error-states branch from 93f1f0b to 43cc2ed Compare April 19, 2021 10:12

zwliew changed the base branch from develop to frontend-tests-dashboard April 19, 2021 10:12

miazima reviewed Apr 20, 2021

View reviewed changes

zwliew mentioned this pull request Apr 22, 2021

feat: refactor msg template components; add telegram character limit #1148

Merged

1 task

Base automatically changed from frontend-tests-dashboard to develop April 26, 2021 04:39

zwliew added 14 commits April 26, 2021 15:21

chore(travis): run the frontend tests

f31cd08

chore(amplify): run the frontend tests

7c68bb2

Make sure to set CI=true explicitly as Amplify doesn't set it by default. CI=true will run all the tests exactly once and exit. Also compile translations before running the tests.

test(app): add an initial render test for App.tsx

c587fb3

Just a simple render test to ensure that the test infrastructure works fine.

fix(amplify): fix test configuration

050b791

fix(travis): run frontend tests only after building

d755b58

This makes more sense as compilation errors are more important than test errors. This also matches the Amplify build configuration.

feat(app.test): add checks for the footer; improve docs

61f23e6

feat(dashboard): write initial email campaign test

e10507b

feat(dashboard): add initial SMS campaign workflow test

f8d1b4e

Also refactor common API endpoints shared with the Email campaign test into a separate array.

feat(dashboard): test the flow for actually creating a new campaign

8421eaa

feat(dashboard): add test for telegram campaign creation workflow

906227a

Also add assertions for testing the credential dropdown for SMS campaigns.

fix(email): select the correct channel button when creating email cam…

7039d30

…paign Previously, we were selecting the SMS channel button, not the email button.

feat(dashboard): colocate APIs; implement initial campaign management

982c477

feat(dashboard): add happy path test for protected email campaign

7707135

chore(test): rename dashboard integration tests for clarity

5df18c0

zwliew added 19 commits April 26, 2021 15:22

feat(email): add unit test for invalid subject

76c6f15

fix(test-utils/api): extract params using the template client

4203098

fix(test-utils/api): store the sanitized email subject and body

03f4079

feat(test-utils/api): add template var checks for protected emails

8e3077c

feat: add unit tests for saving invalid protected email templates

de0aa7e

feat: add a simple render test for EmailRecipients

2a30995

feat: simulate the validation of invalid credentials

417f586

feat: add unit test for invalid SMS credentials

2b26014

chore: add comment headers for setup and teardown sections

2a17af1

feat: add unit test for validating an invalid Telegram credential

f94cf21

fix: query credential label options by role

1006182

feat: add unit tests for protected email error states

6dbd3fa

fix: fix types after rebasing on updated API mocks

f39cae6

fix: add missing check for the custom from address dropdown

60d50a2

feat: mock API responses for invalid CSV files

74b94ea

refactor: move CSV file constants to test-utils

18226dc

refactor: move protected email integration test to a separate file

08d3aab

chore: add tests for uploading invalid CSV files

aefeac7

chore: fix rebase conflicts

b986412

zwliew force-pushed the frontend-tests-error-states branch from e560206 to b986412 Compare April 26, 2021 07:26

zwliew added 2 commits April 26, 2021 15:42

fix: fill invalid CSV files with invalid data

ffad95d

feat: add test for uploading invalid CSV file for protected email

0b6b279

miazima self-requested a review April 27, 2021 04:57

miazima approved these changes Apr 27, 2021

View reviewed changes

miazima merged commit 201957c into develop Apr 27, 2021

miazima deleted the frontend-tests-error-states branch April 27, 2021 04:58

zwliew mentioned this pull request Apr 28, 2021

Write frontend tests for the campaign creation workflow #1079

Closed

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add unit tests for error states in critical workflows #1118

feat: add unit tests for error states in critical workflows #1118

zwliew commented Apr 9, 2021 •

edited

Loading

zwliew commented Apr 19, 2021

miazima left a comment

miazima Apr 20, 2021 •

edited

Loading

zwliew Apr 21, 2021

miazima Apr 26, 2021

zwliew Apr 26, 2021

miazima Apr 26, 2021

miazima Apr 20, 2021

miazima Apr 27, 2021

zwliew Apr 27, 2021 •

edited

Loading

miazima Apr 20, 2021

zwliew Apr 21, 2021 •

edited

Loading

zwliew Apr 22, 2021

miazima Apr 27, 2021

zwliew Apr 27, 2021

zwliew commented Apr 26, 2021

feat: add unit tests for error states in critical workflows #1118

feat: add unit tests for error states in critical workflows #1118

Conversation

zwliew commented Apr 9, 2021 • edited Loading

Problem

Solution

Before & After Screenshots

Tests

Deploy Notes

zwliew commented Apr 19, 2021

miazima left a comment

Choose a reason for hiding this comment

miazima Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwliew Apr 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwliew Apr 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwliew commented Apr 26, 2021

zwliew commented Apr 9, 2021 •

edited

Loading

miazima Apr 20, 2021 •

edited

Loading

zwliew Apr 27, 2021 •

edited

Loading

zwliew Apr 21, 2021 •

edited

Loading