chore(cypress): extend test coverage to include element instance updates #4506
Conversation
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
dosubot
bot
added
the
size:XXL
|
Warning Rate limit exceeded@sjschlapbach has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis change introduces a comprehensive suite of Cypress end-to-end tests that cover the creation, editing, validation, and cleanup of various question types—including content, flashcards, single choice, multiple choice, KPRIM, numerical, free text, selection, and case study questions. Each suite logs in a lecturer, loads fixture data, and simulates user interactions. Additionally, a dedicated set of tests verifies question operations such as duplication, auto-save functionality, and local storage interactions. No modifications were made to any exported or public entity declarations. Changes
Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (15)
cypress/cypress/e2e/D9-questions-case-study-workflow.cy.ts (3)
37-37: Avoid arbitrary wait times to improve reliability.
Relying on fixed time delays (e.g., cy.wait(500) or cy.wait(1000)) can lead to flaky tests. Instead, consider using Cypress commands like .should('exist') or .should('be.visible') to wait on specific UI states before proceeding.Also applies to: 187-187
296-296: Typographical correction in comment.
The comment "check that sliders are initilized" has a minor typo. Use "initialized" for clarity.- // check that sliders are initilized correctly + // check that sliders are initialized correctly
83-135: Consider modularizing repetitive logic.
Blocks of code such as adding/removing criteria or iterating over cases and items appear multiple times. Extracting these into custom Cypress commands or helper functions can improve readability and maintainability.Also applies to: 153-181, 221-255, 318-348
cypress/cypress/e2e/D1-questions-content-workflow.cy.ts (1)
34-35: Avoid relying on time-based waits when possible.
Calls to cy.wait(1000) can introduce unnecessary flakiness. Consider waiting for specific UI states (e.g., an element’s visibility or request completion) for more robust tests.Also applies to: 79-80
cypress/cypress/e2e/D2-questions-flashcards-workflow.cy.ts (1)
37-38: Minimize hard-coded waits.
Avoiding fixed time waits (e.g., cy.wait(1000)) can prevent test instability. Poll for the needed state or event instead.Also applies to: 89-90
cypress/cypress/e2e/D7-questions-free-text-workflow.cy.ts (1)
39-47: Enhance assertion messages for better debugging.Add descriptive error messages to assertions to make test failures more informative.
- cy.get(`[data-cy="element-item-${this.data.FT.title}"]`).contains( - this.data.FT.content - ) + cy.get(`[data-cy="element-item-${this.data.FT.title}"]`) + .contains(this.data.FT.content) + .should('be.visible', { message: 'Free text question content should be visible' })cypress/cypress/e2e/D6-questions-numerical-workflow.cy.ts (2)
13-122: Extract common setup into custom commands.The question creation steps are repeated across test files. Consider creating custom commands for common operations.
// In commands.ts Cypress.Commands.add('createNumericalQuestion', (data) => { cy.get('[data-cy="create-question"]').click() cy.get('[data-cy="select-question-type"]').click() cy.get(`[data-cy="select-question-type-${messages.shared.NUMERICAL.typeLabel}"]`).click() // ... rest of the setup }) // In test file cy.createNumericalQuestion(this.data.NR)
135-150: Consider using data-driven tests for range validation.The range validation tests could be more maintainable using a data-driven approach.
const testCases = [ { min: null, max: 10 }, { min: 5, max: null }, // ... more cases ] testCases.forEach(({ min, max }) => { it(`validates range with min=${min} and max=${max}`, () => { // Test implementation }) })cypress/cypress/e2e/D3-questions-sc-workflow.cy.ts (2)
173-173: Improve test description clarity.Test descriptions should clearly indicate the behavior being tested.
- it('Edit the single choice question again and add answer feedbacks' + it('validates that answer feedbacks are required and correctly linked to answer options'
180-186: Extract feedback validation into helper functions.The feedback validation logic is repeated. Consider extracting it into helper functions.
const validateFeedback = (index: number, feedback: string) => { cy.get(`[data-cy="insert-answer-feedback-${index}"]`) .realClick() .type(feedback) .contains(feedback) }Also applies to: 283-287
cypress/cypress/e2e/D4-questions-mc-workflow.cy.ts (1)
227-233: Use custom commands for repetitive assertions.The feedback validation pattern is repeated across test files. Consider creating a custom command.
// In commands.ts Cypress.Commands.add('validateQuestionFeedback', (index: number, feedback: string) => { cy.get(`[data-cy="insert-answer-feedback-${index}"]`) .realClick() .type(feedback) .contains(feedback) }) // In test file cy.validateQuestionFeedback(ix, feedback)Also applies to: 273-277
cypress/cypress/e2e/D10-questions-operations-workflow.cy.ts (3)
27-28: Replace fixed wait times with proper assertions.Instead of using
cy.wait(500), consider using Cypress's built-in retry-ability with assertions to wait for elements to reach the desired state.-cy.wait(500) +cy.get('[data-cy="insert-answer-field-1"]').should('be.visible') -cy.wait(500) +cy.findByText(messages.manage.questionForms.DUPLICATETitle).should('be.visible') -cy.wait(500) +cy.get(`[data-cy="element-item-${this.data.duplication.title}"]`).should('be.visible') -cy.wait(500) +cy.get(`[data-cy="element-item-${this.data.duplication.title}"]`).should('not.exist')Also applies to: 36-37, 38-39, 56-57
98-112: Consider combining related auto-save test cases.The auto-save test cases for empty questions, non-empty questions, and discarding changes could be combined into a single test case to improve test execution time while maintaining the same coverage.
Example structure:
it('Verifies auto-save functionality for different scenarios', function () { // Test empty questions // Test non-empty questions // Test discarding changes })Also applies to: 114-137, 139-150
346-359: Enhance database verification specificity.The database verification could be more specific about which collections should remain after cleanup.
-cy.task('verifyDeletionAnswerCollections').then((result) => { +cy.task('verifyDeletionAnswerCollections', { + expectedCollections: ['seeded_collection_1', 'seeded_collection_2'] +}).then((result) => {cypress/cypress/e2e/D8-questions-selection-workflow.cy.ts (1)
326-337: Apply DRY principle to repeated verification logic.The verification of answer options deletion and editing permissions is duplicated. Consider extracting this into a helper function.
function verifyAnswerOptionPermissions(solutions: string[], shouldBeDisabled: boolean) { cy.wrap(solutions).each((sol: string) => { cy.get(`[data-cy="delete-answer-option-${sol}"]`) .should(shouldBeDisabled ? 'be.disabled' : 'not.be.disabled') cy.get(`[data-cy="edit-answer-option-${sol}"]`) .should('not.be.disabled') }) }Also applies to: 342-352
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cypress/cypress/e2e/D1-questions-content-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D10-questions-operations-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D2-questions-flashcards-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D3-questions-sc-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D4-questions-mc-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D5-questions-kprim-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D6-questions-numerical-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D7-questions-free-text-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D8-questions-selection-workflow.cy.ts(1 hunks)cypress/cypress/e2e/D9-questions-case-study-workflow.cy.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: cypress-run
- GitHub Check: SonarCloud
🔇 Additional comments (8)
cypress/cypress/e2e/D9-questions-case-study-workflow.cy.ts (2)
351-374: Restricting deletion based on usage looks good.
All checks disabling the delete button for answer options currently in use help prevent accidental data loss. The approach is consistent and clear.Also applies to: 867-905
642-665: Ensure consistent test flow when switching collections.
The flow of canceling and then confirming the change of collections is well covered. Just be aware that changing the collection resets related data, so continuing to check the “disabled” state thoroughly is a good practice.Also applies to: 668-782
cypress/cypress/e2e/D1-questions-content-workflow.cy.ts (2)
13-45: Overall creation and editing workflows look consistent.
The tests efficiently cover basic content creation, status selection, and text editing. They’re straightforward and should be easy to maintain.Also applies to: 65-90
108-114: Cleanup steps are properly scoped.
Removing the content element in a dedicated test ensures the environment is reset. This approach helps avoid pollution between test runs.cypress/cypress/e2e/D2-questions-flashcards-workflow.cy.ts (2)
13-48: Well-structured flashcard creation and validation.
Test covers crucial fields (title, status, content, explanation) and ensures they’re displayed and stored properly.Also applies to: 50-69
71-100: Editing workflow is thorough.
The step-by-step approach to modifying each field and revalidating final results is robust and readable. Good job.Also applies to: 102-118
cypress/cypress/e2e/D10-questions-operations-workflow.cy.ts (1)
73-96: Well-structured helper function!The
enterSCQuestionContenthelper function is well-organized and promotes code reuse across tests.cypress/cypress/e2e/D5-questions-kprim-workflow.cy.ts (1)
1-361: Well-structured test suite with comprehensive coverage!The test suite effectively covers all aspects of KPRIM questions including creation, validation, editing, and cleanup.
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
element-instance-updates-tests
|
| Run status |
|
| Run duration | 25m 03s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
349
|
| View all changes introduced in this branch ↗︎ | |
…esponding validations
…zh-bf/klicker-uzh into element-instance-updates-tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
cypress/cypress/support/commands.ts (1)
255-267: Consider extracting feedback configuration to a helper function.The feedback configuration logic could be extracted to improve maintainability and reusability across different question types (SC, MC, KPRIM).
+function configureChoiceFeedbacks(choices: { feedback: string }[]) { + cy.get('[data-cy="configure-answer-feedbacks"]').click() + + cy.wrap(choices).each((choice: { feedback: string }, ix) => { + cy.get(`[data-cy="insert-answer-feedback-${ix}"]`) + .realClick() + .type(choice.feedback) + cy.get(`[data-cy="insert-answer-feedback-${ix}"]`).contains( + choice.feedback + ) + }) +} if (choices.every((choice) => typeof choice.feedback !== 'undefined')) { - cy.get('[data-cy="configure-answer-feedbacks"]').click() - - cy.wrap(choices).each((choice: { feedback: string }, ix) => { - cy.get(`[data-cy="insert-answer-feedback-${ix}"]`) - .realClick() - .type(choice.feedback) - cy.get(`[data-cy="insert-answer-feedback-${ix}"]`).contains( - choice.feedback - ) - }) + configureChoiceFeedbacks(choices) }cypress/cypress/e2e/D10-questions-operations-workflow.cy.ts (3)
3-4: Consider using a test constant for the current year.Instead of using
new Date().getFullYear(), consider using a fixed test constant to ensure test stability across different years.-// global variable for ensured consistency with current dates -const currentYear = new Date().getFullYear() +// fixed test constant for date consistency +const TEST_YEAR = 2025
76-99: Extract helper function for repetitive test setup.The
enterSCQuestionContenthelper function could be moved to a shared test utilities file for reuse across different test suites.
408-409: Consider reducing wait times in tests.Multiple occurrences of
cy.wait(500)could be replaced with more reliable wait strategies like waiting for specific elements or network requests to complete.-cy.wait(500) +cy.get(`[data-cy="insert-answer-field-${ix + 1}"]`).should('exist')Also applies to: 409-410
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/frontend-manage/src/components/questions/manipulation/InstanceUpdateSwitch.tsx(1 hunks)cypress/cypress/e2e/D10-questions-operations-workflow.cy.ts(1 hunks)cypress/cypress/e2e/F-live-quiz-workflow.cy.ts(0 hunks)cypress/cypress/fixtures/D-questions.json(1 hunks)cypress/cypress/support/commands.ts(2 hunks)packages/shared-components/src/evaluation/ChoiceFeedback.tsx(2 hunks)packages/shared-components/src/questions/SCAnswerOptions.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- cypress/cypress/e2e/F-live-quiz-workflow.cy.ts
✅ Files skipped from review due to trivial changes (1)
- apps/frontend-manage/src/components/questions/manipulation/InstanceUpdateSwitch.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: cypress-run
- GitHub Check: check
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (6)
packages/shared-components/src/evaluation/ChoiceFeedback.tsx (1)
8-12: LGTM! Props added for testing purposes.The addition of
elementIxandchoiceIxprops enhances testability by providing unique identifiers for feedback instances. The props are correctly used in the data-cy attribute.Also applies to: 14-18, 35-38
packages/shared-components/src/questions/SCAnswerOptions.tsx (1)
89-93: LGTM! Props correctly passed to ChoiceFeedback component.The component properly passes the required props to ChoiceFeedback:
- elementIx from props
- choice.ix as choiceIx
cypress/cypress/support/commands.ts (1)
191-197: LGTM! Interface updated to support feedback.The
CreateChoicesQuestionArgsinterface correctly adds optional feedback for choices.cypress/cypress/e2e/D10-questions-operations-workflow.cy.ts (1)
915-928: Verify course selection error handling.Add test cases to verify error handling when course selection fails or when no courses are available.
cypress/cypress/fixtures/D-questions.json (2)
334-376: New "update" Section Added: Schema & Structure VerificationThis new section extends the fixture with additional fields for updating single choice questions and various quiz types. The naming conventions (e.g., title1, content1, liveQuiz1, etc.) are clear and seem aligned with the test requirements. Please confirm that the resulting schema aligns with the expectations in your Cypress tests and backend data models.
357-362: Schema Consistency Check: "choices3" Array Lacks "feedback" FieldWhile the "choices1" and "choices2" arrays include both "content" and "feedback" properties (with some choices marked as correct), the "choices3" objects only provide the "content" property. If all choices are expected to contain feedback, please verify whether the omission in "choices3" is intentional; otherwise, consider adding the "feedback" field to maintain consistency.
|
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3
|
| Run status |
|
| Run duration | 25m 15s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
349
|
| View all changes introduced in this branch ↗︎ | |
Summary by CodeRabbit