From 6d908927b9ecd01abe3b3bb45f5cf5fb67193462 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Tue, 11 Jun 2024 12:49:02 -0600 Subject: [PATCH] make e2e tests more reliable --- app/utils/user-validation.ts | 5 +++-- package-lock.json | 28 ++++++++++++++++++++++++++++ package.json | 1 + tests/e2e/onboarding.test.ts | 18 ++++++++---------- tests/e2e/send.test.ts | 13 +++++-------- tests/e2e/settings-profile.test.ts | 8 ++++---- tests/mocks/utils.ts | 16 ++++++++++++---- 7 files changed, 61 insertions(+), 28 deletions(-) diff --git a/app/utils/user-validation.ts b/app/utils/user-validation.ts index 8a96c75..7663cd6 100644 --- a/app/utils/user-validation.ts +++ b/app/utils/user-validation.ts @@ -8,7 +8,7 @@ export const UsernameSchema = z message: 'Username can only include letters, numbers, and underscores', }) // users can type the username in any case, but we store it in lowercase - .transform(value => value.toLowerCase()) + .transform(value => value.trim().toLowerCase()) export const PasswordSchema = z .string({ required_error: 'Password is required' }) @@ -22,7 +22,8 @@ export const NameSchema = z export const PhoneNumberSchema = z .string({ required_error: 'Phone number is required' }) .min(3, { message: 'Phone number is too short' }) - .max(20, { message: 'Phone number is too long' }) + .max(30, { message: 'Phone number is too long' }) + .transform(value => value.trim()) export const PasswordAndConfirmPasswordSchema = z .object({ password: PasswordSchema, confirmPassword: PasswordSchema }) diff --git a/package-lock.json b/package-lock.json index 4d5535c..d2fc4f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -110,6 +110,7 @@ "enforce-unique": "^1.3.0", "esbuild": "^0.21.4", "eslint": "^9.4.0", + "filenamify": "^6.0.0", "fs-extra": "^11.2.0", "jsdom": "^24.1.0", "msw": "2.3.1", @@ -12814,6 +12815,33 @@ "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==", "license": "MIT" }, + "node_modules/filename-reserved-regex": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/filename-reserved-regex/-/filename-reserved-regex-3.0.0.tgz", + "integrity": "sha512-hn4cQfU6GOT/7cFHXBqeBg2TbrMBgdD0kcjLhvSQYYwm3s4B6cjvBfb7nBALJLAXqmU5xajSa7X2NnUud/VCdw==", + "dev": true, + "engines": { + "node": "^12.20.0 || ^14.13.1 || >=16.0.0" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/filenamify": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/filenamify/-/filenamify-6.0.0.tgz", + "integrity": "sha512-vqIlNogKeyD3yzrm0yhRMQg8hOVwYcYRfjEoODd49iCprMn4HL85gK3HcykQE53EPIpX3HcAbGA5ELQv216dAQ==", + "dev": true, + "dependencies": { + "filename-reserved-regex": "^3.0.0" + }, + "engines": { + "node": ">=16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/fill-range": { "version": "7.1.1", "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.1.1.tgz", diff --git a/package.json b/package.json index ba3c9a5..42d983f 100644 --- a/package.json +++ b/package.json @@ -143,6 +143,7 @@ "enforce-unique": "^1.3.0", "esbuild": "^0.21.4", "eslint": "^9.4.0", + "filenamify": "^6.0.0", "fs-extra": "^11.2.0", "jsdom": "^24.1.0", "msw": "2.3.1", diff --git a/tests/e2e/onboarding.test.ts b/tests/e2e/onboarding.test.ts index 0854262..d323ce1 100644 --- a/tests/e2e/onboarding.test.ts +++ b/tests/e2e/onboarding.test.ts @@ -1,8 +1,8 @@ import { invariant } from '@epic-web/invariant' import { faker } from '@faker-js/faker' import { prisma } from '#app/utils/db.server.ts' -import { readText } from '#tests/mocks/utils.ts' -import { createUser, expect, test as base } from '#tests/playwright-utils.ts' +import { waitForText } from '#tests/mocks/utils.ts' +import { test as base, createUser, expect } from '#tests/playwright-utils.ts' const URL_REGEX = /(?https?:\/\/[^\s$.?#].[^\s]*)/ const CODE_REGEX = /code: (?[\d\w]+)/ @@ -62,8 +62,7 @@ test('onboarding with link', async ({ page, getOnboardingData }) => { const sourceNumber = await prisma.sourceNumber.findFirstOrThrow({ select: { phoneNumber: true }, }) - const textMessage = await readText(onboardingData.phoneNumber) - invariant(textMessage, 'Text message not found') + const textMessage = await waitForText(onboardingData.phoneNumber) expect(textMessage.To).toBe(onboardingData.phoneNumber.toLowerCase()) expect(textMessage.From).toBe(sourceNumber.phoneNumber) expect(textMessage.Body).toMatch(/welcome/i) @@ -127,8 +126,9 @@ test('onboarding with a short code', async ({ page, getOnboardingData }) => { const sourceNumber = await prisma.sourceNumber.findFirstOrThrow({ select: { phoneNumber: true }, }) - const textMessage = await readText(onboardingData.phoneNumber) - invariant(textMessage, 'Text message not found') + const textMessage = await waitForText(onboardingData.phoneNumber, { + errorMessage: 'Onboarding code not found', + }) expect(textMessage.To).toBe(onboardingData.phoneNumber) expect(textMessage.From).toBe(sourceNumber.phoneNumber) expect(textMessage.Body).toMatch(/welcome/i) @@ -176,8 +176,7 @@ test('reset password with a link', async ({ page, insertNewUser }) => { const sourceNumber = await prisma.sourceNumber.findFirstOrThrow({ select: { phoneNumber: true }, }) - const textMessage = await readText(user.phoneNumber) - invariant(textMessage, 'Text message not found') + const textMessage = await waitForText(user.phoneNumber) expect(textMessage.Body).toMatch(/password reset/i) expect(textMessage.To).toBe(user.phoneNumber) expect(textMessage.From).toBe(sourceNumber.phoneNumber) @@ -237,8 +236,7 @@ test('reset password with a short code', async ({ page, insertNewUser }) => { const sourceNumber = await prisma.sourceNumber.findFirstOrThrow({ select: { phoneNumber: true }, }) - const textMessage = await readText(user.phoneNumber) - invariant(textMessage, 'Text message not found') + const textMessage = await waitForText(user.phoneNumber) expect(textMessage.Body).toMatch(/password reset/i) expect(textMessage.To).toBe(user.phoneNumber) expect(textMessage.From).toBe(sourceNumber.phoneNumber) diff --git a/tests/e2e/send.test.ts b/tests/e2e/send.test.ts index 86104a5..b5f3eb0 100644 --- a/tests/e2e/send.test.ts +++ b/tests/e2e/send.test.ts @@ -1,12 +1,11 @@ -import { invariant } from '@epic-web/invariant' import { faker } from '@faker-js/faker' import { prisma } from '#app/utils/db.server.ts' -import { readText } from '#tests/mocks/utils.ts' +import { waitForText } from '#tests/mocks/utils.ts' import { + createMessage, + createRecipient, expect, test, - createRecipient, - createMessage, waitFor, } from '#tests/playwright-utils.ts' @@ -48,8 +47,7 @@ test('Users can write and send a message immediately', async ({ const sourceNumber = await prisma.sourceNumber.findFirstOrThrow({ select: { phoneNumber: true }, }) - const textMessage = await readText(recipient.phoneNumber) - invariant(textMessage, 'Text message not found') + const textMessage = await waitForText(recipient.phoneNumber) expect(textMessage.To).toBe(recipient.phoneNumber) expect(textMessage.From).toBe(sourceNumber.phoneNumber) @@ -116,8 +114,7 @@ test('Scheduled messages go out on schedule', async ({ page, login }) => { const sourceNumber = await prisma.sourceNumber.findFirstOrThrow({ select: { phoneNumber: true }, }) - const textMessage = await readText(recipient.phoneNumber) - invariant(textMessage, 'Text message not found') + const textMessage = await waitForText(recipient.phoneNumber) expect(textMessage.To).toBe(recipient.phoneNumber) expect(textMessage.From).toBe(sourceNumber.phoneNumber) diff --git a/tests/e2e/settings-profile.test.ts b/tests/e2e/settings-profile.test.ts index c54b70c..583b359 100644 --- a/tests/e2e/settings-profile.test.ts +++ b/tests/e2e/settings-profile.test.ts @@ -2,8 +2,8 @@ import { invariant } from '@epic-web/invariant' import { faker } from '@faker-js/faker' import { verifyUserPassword } from '#app/utils/auth.server.ts' import { prisma } from '#app/utils/db.server.ts' -import { readText } from '#tests/mocks/utils.ts' -import { expect, test, createUser, waitFor } from '#tests/playwright-utils.ts' +import { waitForText } from '#tests/mocks/utils.ts' +import { createUser, expect, test } from '#tests/playwright-utils.ts' const CODE_REGEX = /Here's your verification code: (?[\d\w]+)/ @@ -63,7 +63,7 @@ test('Users can change their phone number', async ({ page, login }) => { .fill(newPhoneNumber) await page.getByRole('button', { name: /send confirmation/i }).click() await expect(page.getByText(/check your texts/i)).toBeVisible() - const text = await waitFor(() => readText(newPhoneNumber), { + const text = await waitForText(newPhoneNumber, { errorMessage: 'Confirmation text message was not sent', }) invariant(text, 'Text was not sent') @@ -80,7 +80,7 @@ test('Users can change their phone number', async ({ page, login }) => { }) invariant(updatedUser, 'Updated user not found') expect(updatedUser.phoneNumber).toBe(newPhoneNumber) - const noticeText = await waitFor(() => readText(preUpdateUser.phoneNumber), { + const noticeText = await waitForText(preUpdateUser.phoneNumber, { errorMessage: 'Notice text was not sent', }) expect(noticeText.Body).toContain('changed') diff --git a/tests/mocks/utils.ts b/tests/mocks/utils.ts index 321e2f7..4a79fd9 100644 --- a/tests/mocks/utils.ts +++ b/tests/mocks/utils.ts @@ -1,7 +1,9 @@ import path from 'node:path' import { fileURLToPath } from 'node:url' +import filenamify from 'filenamify' import fsExtra from 'fs-extra' import { z } from 'zod' +import { waitFor } from '#tests/playwright-utils.js' const __dirname = path.dirname(fileURLToPath(import.meta.url)) const fixturesDirPath = path.join(__dirname, '..', 'fixtures') @@ -28,7 +30,7 @@ export const TextMessageSchema = z.object({ export async function writeText(rawText: unknown) { const textMessage = TextMessageSchema.parse(rawText) - await createFixture('texts', textMessage.To, textMessage) + await createFixture('texts', filenamify(textMessage.To), textMessage) return textMessage } @@ -40,14 +42,20 @@ export async function requireText(recipient: string) { export async function readText(recipient: string) { try { - const textMessage = await readFixture('texts', recipient) + const textMessage = await readFixture('texts', filenamify(recipient)) return TextMessageSchema.parse(textMessage) - } catch (error) { - console.error(`Error reading text message to ${recipient}`, error) + } catch { return null } } +export async function waitForText( + recipient: string, + options: Parameters[1] = {}, +) { + return waitFor(() => requireText(recipient), options) +} + export function requireHeader(headers: Headers, header: string) { if (!headers.has(header)) { const headersString = JSON.stringify(