-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore(dashboard): user journey smoke tests #7124
base: next
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
apps/api/.gitignore
Outdated
@@ -121,3 +121,4 @@ backups/ | |||
|
|||
# Nest.js auto-generated metadata (https://docs.nestjs.com/recipes/swc#monorepo-and-cli-plugins) | |||
src/metadata.ts | |||
src/.env.test |
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.
removed the .env.test
file from git, because the local file that I used for the tests had sensitive clerk credentials
@@ -5,6 +5,7 @@ | |||
"type": "module", | |||
"scripts": { | |||
"start": "vite", | |||
"start:test": "vite --mode test", |
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.
run dashboard and use .env.test
file
"@novu/dal": "workspace:*", | ||
"@novu/testing": "workspace:*", |
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.
used to initialize the api session
const fileName = fileURLToPath(import.meta.url); | ||
const dirName = dirname(fileName); | ||
dotenv.config({ path: path.resolve(dirName, '.env.playwright') }); |
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.
Because the Dashboard project is configured with "type": "module"
this is how we can import the playwright env variables.
apps/dashboard/playwright.config.ts
Outdated
{ | ||
name: 'setup', | ||
testMatch: /global\.setup\.ts/, | ||
}, |
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 global setup test file will initialize the Clerk session for all tests. Using this approach, we can say that for some tests, we don't want to have a session, and then we will be able to verify, for example, the sign-in/up flows.
bridgeServer = new BridgeServer({ secretKey: session.environment.apiKeys[0].key, apiUrl: process.env.API_URL }); | ||
|
||
const newWorkflow = workflow(workflowId, async ({ step }) => { | ||
await step.inApp( | ||
inAppStepId, | ||
async (controls) => { | ||
return { | ||
subject: `Hi ${controls.name}! You've been invited to join the Novu project`, | ||
body, | ||
}; | ||
}, | ||
{ | ||
controlSchema: { | ||
type: 'object', | ||
properties: { | ||
name: { type: 'string', default: 'John' }, | ||
}, | ||
} as const, | ||
} | ||
); | ||
}); | ||
await bridgeServer.start({ workflows: [newWorkflow] }); | ||
await session.testAgent.post(`/v1/bridge/sync`).send({ | ||
bridgeUrl: bridgeServer.serverPath, | ||
}); |
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.
Run the bridge server and sync the workflow
await session.testAgent.post(`/v1/bridge/sync`).send({ | ||
bridgeUrl: bridgeServer.serverPath, | ||
}); | ||
await page.waitForTimeout(2000); |
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.
should be removed
apps/dashboard/tests/global.setup.ts
Outdated
/** | ||
* TODO: Currently we are running tests with a single Clerk user and organization. | ||
* This approach has a drawback that the tests are not independent from each other and write the data to the same organization. | ||
* We should consider creating a new Clerk user and organization for each test using the @clerk/backend package. | ||
* Then initialize the BE session with the new Clerk user and organization and update Clerk user metadata from the newly created session. | ||
*/ |
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.
hopefully, it explains the problem well
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.
Correct! We should leverage Clerk Backend API, create a user before every run, sign in with that user, and then delete the user at the end.
Regarding the need for Clerk webhooks during user and organization creation, I suggest connecting to Mongo directly from the test code using the response of the Clerk API to create the user and organization record the application needs to function.
On user deletion, I don't think we need to do anything in Mongo as we run in a dockerized environment.
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'll think about this, as it will require reimplementing all the current setup.
@@ -0,0 +1,3 @@ | |||
{ | |||
"type": "commonjs" |
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 dal
and testing
packages are CJS modules which can't be imported by the ESM modules.
So this is the trick (together with tsconfig.json) to say "please allow me to import" in my test files :D
private memberRepository = getEERepository<MemberRepository>('MemberRepository'); | ||
private memberRepository: MemberRepository; | ||
|
||
constructor({ mockClerkClient = true }: { mockClerkClient?: 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.
For the Dashboard E2E tests we don't want to mock the Clerk client, because we initialize the Clerk session with a real user that has the org and metadata that is used by the Dashboard code.
@novu/client
@novu/framework
@novu/js
@novu/nest
@novu/headless
@novu/nextjs
@novu/node
@novu/notification-center
novu
@novu/providers
@novu/react
@novu/react-native
@novu/shared
@novu/stateless
commit: |
…nd sdk to create and sign-in test users
touch .env | ||
echo VITE_LAUNCH_DARKLY_CLIENT_SIDE_ID=${{ secrets.LAUNCH_DARKLY_CLIENT_SIDE_ID }} >> .env | ||
echo VITE_API_HOSTNAME=http://127.0.0.1:1336 >> .env | ||
echo VITE_WEBSOCKET_HOSTNAME=http://127.0.0.1:1340 >> .env | ||
echo VITE_LEGACY_DASHBOARD_URL=http://127.0.0.1:4200 >> .env | ||
echo VITE_CLERK_PUBLISHABLE_KEY=${{ secrets.CLERK_E2E_PUBLISHABLE_KEY }} >> .env |
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.
.env file for the Dashboard
touch .env.playwright | ||
echo NOVU_ENTERPRISE=true >> .env.playwright | ||
echo NEW_RELIC_ENABLED=false >> .env.playwright | ||
echo NEW_RELIC_APP_NAME=Novu >> .env.playwright | ||
echo MONGO_URL=mongodb://127.0.0.1:27017/novu-test >> .env.playwright | ||
echo API_URL=http://127.0.0.1:1336 >> .env.playwright | ||
echo CLERK_ENABLED=true >> .env.playwright | ||
echo CLERK_PUBLISHABLE_KEY=${{ secrets.CLERK_E2E_PUBLISHABLE_KEY }} >> .env.playwright | ||
echo CLERK_SECRET_KEY=${{ secrets.CLERK_E2E_SECRET_KEY }} >> .env.playwright | ||
echo NODE_ENV=test >> .env.playwright |
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.
.env file for the playwright, used to initialize the session, create, and sync users/orgs
"@clerk/backend": "^1.6.2", | ||
"@clerk/testing": "^1.3.27", |
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.
used to create Clerk user/orgs and to sign-in into the app
e.preventDefault(); | ||
e.stopPropagation(); |
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.
fix the bug - the click on remove button was causing the form submission
@@ -80,7 +80,7 @@ export const NodeBody = ({ | |||
|
|||
return ( | |||
<HoverCard openDelay={300}> | |||
<HoverCardTrigger> | |||
<HoverCardTrigger asChild> |
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.
react warning a
nested in a
return () => { | ||
observer.disconnect(); | ||
}; |
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.
react warning on that the callback is never called
What changed? Why was the change needed?
Enterprise PR: https://github.com/novuhq/packages-enterprise/pull/273
Dashboard - the core user journeys smoke tests that cover:
Screenshots