-
Notifications
You must be signed in to change notification settings - Fork 301
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
Development
: Add e2e playwright tests for exam announcement
#8630
Development
: Add e2e playwright tests for exam announcement
#8630
Conversation
This reverts commit 08e7d09.
…rticipation actions into a separate fixture
WalkthroughThe recent updates primarily focus on enhancing the Playwright test suite for exam-related functionalities. Key changes include renaming the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 2
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.
Demonstrated in testing session, Code also LGTM
async checkExamFinishedTitle(title: string) { | ||
await expect(this.page.locator('#exam-finished-title')).toContainText(title, { timeout: 40000 }); | ||
} | ||
|
||
async checkExamFullnameInputExists() { | ||
await expect(this.page.locator('#fullname')).toBeVisible({ timeout: 30000 }); | ||
} | ||
|
||
async checkYourFullname(name: string) { | ||
await expect(this.page.locator('#your-name')).toContainText(name, { timeout: 30000 }); | ||
} |
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.
Why are the timeouts so long? This would be 40/30 seconds, right?
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 checked and turns out these are timeouts for waiting for an exam to finish, delegated to POM functions. Would be better to wait for exam finish explicitly in the test if needed, not in an assertion coming after.
But these functions are not relevant to current PR, I just moved them to different POM. Maybe I should fix that in another PR?
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.
yes, please fix it in another PR
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.
Demonstrated in a testing session.
ffcdb3a
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.
re-approve
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.
re-approve
(last one was a misclick :'))
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.
demonstrated in the testing session, looks good. I think you could also add another e2e test for problem statement updates in a follow-up PR.
Development
: Exam announcement e2e testsDevelopment
: Add e2e playwright tests for exam announcement
Checklist
General
Client
Motivation and Context
We want to test exam announcement dialogs popping up by actions such as instructor sending a message or exam working time change.
Description
Adds E2E tests in Playwright about exam announcements.
Steps for Testing
Steps for running the tests:
npm install && npm run playwright:setup && npx playwright test e2e/exam/ExamParticipation.spec.ts
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Code Review
Manual Tests