-
Notifications
You must be signed in to change notification settings - Fork 14
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: fix test suite after combination of extended tests with new live quiz URLs #4369
Conversation
📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces modifications to the Cypress end-to-end test suite for course management and live quiz functionalities. Key changes include renaming UI selectors from "sessions" to "quizzes," reflecting updates in the application's structure. The test cases have been expanded to cover more scenarios, particularly around gamification features, with enhanced error handling and assertions. Additionally, minor changes were made to CSS files, including the removal of semicolons in Changes
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
cypress/cypress/e2e/F-live-quiz-workflow.cy.ts (2)
Line range hint
554-558
: Replace arbitrary wait times with proper assertions.Using arbitrary wait times with
cy.wait()
can make tests flaky. Instead, wait for specific elements or states.Consider replacing wait times with proper assertions:
-cy.wait(500) -cy.get('[data-cy="next-block-timeline"]').click() -cy.wait(500) +cy.get('[data-cy="next-block-timeline"]').should('be.enabled').click() +cy.get('[data-cy="block-status"]').should('contain', 'completed')Also applies to: 701-704
Line range hint
644-645
: Address TODO comment about element stacks migration.The TODO comment indicates a pending migration to element stacks that should be addressed.
Would you like me to help create a GitHub issue to track the migration to element stacks? This would help ensure the task isn't forgotten and can be properly prioritized.
🛑 Comments failed to post (1)
cypress/cypress/e2e/F-live-quiz-workflow.cy.ts (1)
273-273: 🛠️ Refactor suggestion
Inconsistent selector naming convention.
While the main navigation selector has been updated from "sessions" to "quizzes", there are still several session-related selectors that should be updated for consistency:
session-block
start-session-
session-cockpit-
abort-session-cockpit
Consider updating these selectors to maintain consistency with the new "quiz" terminology:
-cy.contains('[data-cy="session-block"]', sessionName1) +cy.contains('[data-cy="quiz-block"]', sessionName1) -cy.get(`[data-cy="start-session-${sessionName1New}"]`) +cy.get(`[data-cy="start-quiz-${sessionName1New}"]`) -cy.get(`[data-cy="session-cockpit-${sessionName2}"]`) +cy.get(`[data-cy="quiz-cockpit-${sessionName2}"]`) -cy.get('[data-cy="abort-session-cockpit"]') +cy.get('[data-cy="abort-quiz-cockpit"]')Also applies to: 492-492, 526-526, 536-536, 571-571, 700-700, 712-712, 766-766, 791-791, 799-799
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/frontend-manage/src/components/sessions/Session.tsx (1)
Line range hint
195-204
: Enhance accessibility for interactive elements.While the component uses semantic HTML well, consider these accessibility improvements:
- Add aria-label to the start session button
- Ensure loading state is announced to screen readers
<Button basic disabled={startingQuiz} + aria-label={t('manage.sessions.startSession')} + aria-busy={startingQuiz} onClick={async () => { await startSession() router.push(`quizzes/${session.id}/cockpit`) }} data={{ cy: `start-session-${session.name}` }} >
🛑 Comments failed to post (1)
apps/frontend-manage/src/components/sessions/Session.tsx (1)
195-197: 🛠️ Refactor suggestion
Add error handling for session start and navigation.
The current implementation lacks error handling for both the session start mutation and navigation. Consider adding try-catch and user feedback:
- onClick={async () => { - await startSession() - router.push(`quizzes/${session.id}/cockpit`) + onClick={async () => { + try { + await startSession() + await router.push(`quizzes/${session.id}/cockpit`) + } catch (error) { + // Add error notification/toast here + console.error('Failed to start session:', error) + } }}Committable suggestion skipped: line range outside the PR's diff.
Quality Gate passedIssues Measures |
klicker-uzh Run #3645
Run Properties:
|
Project |
klicker-uzh
|
Branch Review |
fix-test-suite
|
Run status |
Passed #3645
|
Run duration | 11m 13s |
Commit |
e0449dd607 ℹ️: Merge b097e50c3e37e12fa3f9e54ec92ce71de0708a67 into 71aeaa5115d4c1c99ca88d073d05...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
140
|
View all changes introduced in this branch ↗︎ |
Summary by CodeRabbit
New Features
Bug Fixes
Style
User Experience