Development: Improve execution speed of Playwright tests
#9817
Conversation
…again" This reverts commit 56a943c.
This reverts commit 462d0c0.
…t for all. Rename @slow tests to @Sequential to make it clearer.
muradium
changed the title
Feature: Improve execution speed of Playwright tests
muradium
changed the title
Feature: Improve execution speed of Playwright testsDevelopment: Improve execution speed of Playwright tests
WalkthroughThe changes in this pull request primarily involve modifications to the test suites for various exam-related functionalities in the Playwright framework. The updates include adjustments to test categorization through the addition and removal of tags, specifically changing tags from Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/test/playwright/e2e/exam/ExamAssessment.spec.tsOops! Something went wrong! :( ESLint: 9.14.0 TypeError: Error while loading rule '@typescript-eslint/no-unused-expressions': Cannot read properties of undefined (reading 'allowShortCircuit') 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (15)
src/test/playwright/package.json (1)
19-19: Consider adding explicit worker count controlThe parallel execution might benefit from an explicit
--workersparameter to prevent system resource exhaustion. The optimal number could be based on the CI/CD environment's capabilities.- "playwright:test:parallel": "cross-env PLAYWRIGHT_JUNIT_OUTPUT_NAME=./test-reports/results-parallel.xml playwright test e2e --project=fast-tests --project=slow-tests", + "playwright:test:parallel": "cross-env PLAYWRIGHT_JUNIT_OUTPUT_NAME=./test-reports/results-parallel.xml playwright test e2e --project=fast-tests --project=slow-tests --workers=${MAX_WORKERS:-4}",src/test/playwright/e2e/exercise/modeling/ModelingExerciseParticipation.spec.ts (1)
Line range hint
11-31: Consider enhancing test isolation and cleanup robustness.While the test is well-structured, there are opportunities to improve its reliability:
- Consider using unique identifiers in course/exercise names to prevent any potential conflicts in parallel execution
- Add error handling in cleanup to ensure resources are deleted even if tests fail
Here's a suggested improvement:
test.beforeEach('Create course', async ({ login, courseManagementAPIRequests, exerciseAPIRequests }) => { await login(admin); - course = await courseManagementAPIRequests.createCourse(); + // Add unique identifier to prevent conflicts in parallel execution + course = await courseManagementAPIRequests.createCourse({ + title: `Modeling Test Course ${Date.now()}-${Math.random().toString(36).substring(7)}` + }); await courseManagementAPIRequests.addStudentToCourse(course, studentOne); modelingExercise = await exerciseAPIRequests.createModelingExercise({ course }); }); test.afterEach('Delete course', async ({ courseManagementAPIRequests }) => { - await courseManagementAPIRequests.deleteCourse(course, admin); + // Ensure cleanup runs even if test fails + try { + if (course?.id) { + await courseManagementAPIRequests.deleteCourse(course, admin); + } + } catch (error) { + console.error('Failed to cleanup test resources:', error); + throw error; + } });src/test/playwright/playwright.config.ts (2)
35-41: Consider adding explicit worker configuration and reviewing test categorization strategy.While the configuration for fast tests looks good overall, consider these improvements:
- Add explicit
workersconfiguration to optimize parallel execution for fast tests- The pattern
^[^@]*$might accidentally include untagged tests that should be categorized. Consider requiring explicit@fasttags for better maintainability.{ name: 'fast-tests', - grep: /@fast|^[^@]*$/, + grep: /@fast/, timeout: (parseNumber(process.env.FAST_TEST_TIMEOUT_SECONDS) ?? 45) * 1000, + workers: parseNumber(process.env.FAST_TEST_WORKERS) ?? 'max', use: { ...devices['Desktop Chrome'] }, },
Line range hint
1-58: Documentation improvements needed for test configuration.Please add comprehensive documentation about:
- Required environment variables and their default values
- Recommended commands for running different test categories
- Migration guide for moving tests between categories
Add this documentation at the top of the file:
import { defineConfig, devices } from '@playwright/test'; import dotenv from 'dotenv'; import { parseNumber } from './support/utils'; + +/** + * Test Configuration Guide + * ----------------------- + * Environment Variables: + * - TEST_TIMEOUT_SECONDS: Global timeout (default: 180) + * - TEST_RETRIES: Number of retries (default: 2) + * - TEST_WORKER_PROCESSES: Global worker count (default: 3) + * - FAST_TEST_TIMEOUT_SECONDS: Timeout for @fast tests (default: 45) + * - SLOW_TEST_TIMEOUT_SECONDS: Timeout for @slow tests (default: 180) + * - SEQUENTIAL_TEST_TIMEOUT_SECONDS: Timeout for @sequential tests (default: 180) + * + * Running Tests: + * - All tests: npx playwright test + * - Fast tests only: npx playwright test --project=fast-tests + * - Sequential tests: npx playwright test --project=sequential-tests + * + * Test Categories: + * - @fast: Quick tests that can run in parallel + * - @slow: Longer running tests that can run in parallel + * - @sequential: Tests that must run sequentially (e.g., submission tests) + */src/test/playwright/e2e/exam/test-exam/TestExamCreationDeletion.spec.ts (1)
Line range hint
33-77: Consider optimizing test performance furtherWhile the test is correctly tagged as '@fast', there are opportunities to make it even faster:
- Consider moving more setup operations to API calls instead of UI interactions
- The exam creation test could potentially be split into:
- A fast API test for validation logic
- A minimal UI test for critical paths only
This would align better with the PR's goal of improving test execution speed.
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseAssessment.spec.ts (1)
Line range hint
94-98: Consider addressing the TODO comment about complaint functionality.The commented code block indicates a pending fix for the accept/reject complaint functionality. This might affect the completeness of the test coverage.
Would you like me to:
- Help implement the fix for the accept/reject complaint functionality?
- Create a GitHub issue to track this TODO?
src/test/playwright/e2e/exam/test-exam/TestExamManagement.spec.ts (1)
14-14: Consider adding custom timeouts for resource-intensive tests.While the suite is correctly tagged as
@fast, the programming exercise creation test might need a longer timeout than other tests in this suite.Consider adding a custom timeout for the programming exercise test:
- test('Adds a programming exercise', async ({ page, examManagement, examExerciseGroups, programmingExerciseCreation }) => { + test('Adds a programming exercise', { timeout: 90000 }, async ({ page, examManagement, examExerciseGroups, programmingExerciseCreation }) => {src/test/playwright/e2e/exam/ExamTestRun.spec.ts (1)
Line range hint
22-45: Consider optimizing test setup for faster executionTo improve test execution speed, consider moving the course and exam creation to a
beforeAllhook instead ofbeforeEach. This would reduce the number of API calls and setup time significantly.- test.beforeEach('Create course', async ({ login, examExerciseGroupCreation, courseManagementAPIRequests, examAPIRequests }) => { + test.beforeAll('Create course', async ({ login, examExerciseGroupCreation, courseManagementAPIRequests, examAPIRequests }) => {Note: Ensure that tests are independent and don't modify the shared resources in a way that affects other tests.
src/test/playwright/e2e/exercise/ExerciseImport.spec.ts (1)
Test categorization needs attention across the codebase
The verification reveals inconsistencies in test categorization:
- Several exam-related tests are still using the deprecated
@slowtag that should be removed- Programming exercise tests in
ExerciseImport.spec.tscorrectly use@sequential, but other programming submission tests lack proper tagging- Non-programming tests are inconsistently tagged with
@fast🔗 Analysis chain
Line range hint
20-150: Verify consistent test categorization across the codebaseLet's ensure that the test categorization pattern is consistently applied across all Playwright test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify test categorization consistency across Playwright tests # Expected patterns: # - Programming submission tests should use @sequential # - Other tests should use @fast # - @slow tag should be removed echo "Checking for any remaining @slow tags..." rg "tag: '@slow'" "src/test/playwright" echo "Checking programming submission tests for @sequential tag..." rg -B2 "makeSubmissionAndVerifyResults|submitSolution" "src/test/playwright" | rg "tag: '@sequential'" echo "Checking non-programming tests for @fast tag..." rg -B2 "submit\(\)|submitExercise\(\)" "src/test/playwright" | rg "tag: '@fast'"Length of output: 1739
Script:
#!/bin/bash # Let's try a more targeted search for test categorization patterns echo "Checking programming exercise tests with their tags..." rg -B3 "programming.*exercise" "src/test/playwright" | rg "test\(.*tag:" echo "Checking all test tags in playwright tests..." rg "test\(.*tag: '@[^']*'" "src/test/playwright" -A1 echo "Checking specifically for programming submission patterns..." rg -B5 "programming.*exercise.*submission" "src/test/playwright"Length of output: 10609
src/test/playwright/e2e/exam/ExamResults.spec.ts (1)
138-168: Fix potential variable scope issues in switch statementThe switch statement contains variable declarations that could leak between cases. While this isn't causing issues currently, it's better to follow best practices and wrap case blocks in curly braces.
Consider refactoring the switch statement like this:
switch (testCase.exerciseType) { case ExerciseType.TEXT: { await examResultsPage.checkTextExerciseContent(exercise.id!, exercise.additionalData!.textFixture!); await examResultsPage.checkAdditionalFeedback(exercise.id!, 7, 'Good job'); break; } case ExerciseType.PROGRAMMING: { await examResultsPage.checkProgrammingExerciseAssessments(exercise.id!, 'Wrong', 7); await examResultsPage.checkProgrammingExerciseAssessments(exercise.id!, 'Correct', 6); const taskStatuses: ProgrammingExerciseTaskStatus[] = [ ProgrammingExerciseTaskStatus.SUCCESS, ProgrammingExerciseTaskStatus.SUCCESS, ProgrammingExerciseTaskStatus.SUCCESS, ProgrammingExerciseTaskStatus.FAILURE, ProgrammingExerciseTaskStatus.FAILURE, ProgrammingExerciseTaskStatus.FAILURE, ProgrammingExerciseTaskStatus.FAILURE, ]; await examResultsPage.checkProgrammingExerciseTasks(exercise.id!, taskStatuses); break; } case ExerciseType.QUIZ: { await examResultsPage.checkQuizExerciseScore(exercise.id!, 5, 10); const studentAnswers = [true, false, true, false]; const correctAnswers = [true, true, false, false]; await examResultsPage.checkQuizExerciseAnswers(exercise.id!, studentAnswers, correctAnswers); break; } case ExerciseType.MODELING: { await examResultsPage.checkAdditionalFeedback(exercise.id!, 5, 'Good'); await examResultsPage.checkModellingExerciseAssessment(exercise.id!, 'class Class', 'Wrong', -1); await examResultsPage.checkModellingExerciseAssessment(exercise.id!, 'abstract class Abstract', 'Neutral', 0); break; } }🧰 Tools
🪛 Biome
[error] 146-154: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 160-160: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts (4)
Line range hint
58-61: Consider improving C language test skip logicThe current skip logic for C tests uses an inline condition. Consider extracting this into a helper function or test tag for better maintainability and clarity.
- if (programmingLanguage !== ProgrammingLanguage.C || process.env.PLAYWRIGHT_DB_TYPE !== 'Postgres') { + const shouldSkipCTest = programmingLanguage === ProgrammingLanguage.C && process.env.PLAYWRIGHT_DB_TYPE === 'Postgres'; + test.skip(shouldSkipCTest, 'C tests are not supported in Postgres setup'); + if (!shouldSkipCTest) {
Line range hint
6-11: Consider using dynamic test data loadingThe test fixtures are imported with hardcoded paths. Consider implementing a dynamic test data loader to improve maintainability and make it easier to add new test cases.
// Example test data loader const loadTestData = (language: ProgrammingLanguage, scenario: string) => import(`../../../fixtures/exercise/programming/${language}/${scenario}/submission.json`);
Line range hint
251-254: Security: Avoid including credentials in Git URLsIncluding passwords in Git URLs is a security risk as they might be logged or exposed. Consider using Git credentials helper or environment variables for authentication.
- repoUrl = repoUrl.replace(student.username!, `${student.username!}:${student.password!}`); - repoUrl = repoUrl.replace(`:**********`, ``); + // Use Git credentials helper + await gitClient.configureAuth(student.username!, student.password!);
Line range hint
257-258: Improve repository cleanup reliabilityThe repository cleanup should be moved to a
try/finallyblock to ensure cleanup even if the test fails.- await fs.rmdir(`./test-exercise-repos/${repoName}`, { recursive: true }); - await page.goto(`courses/${course.id}/exercises/${exercise.id!}`); + try { + await page.goto(`courses/${course.id}/exercises/${exercise.id!}`); + const resultScore = await programmingExerciseOverview.getResultScore(); + await expect(resultScore.getByText(submission.expectedResult)).toBeVisible(); + } finally { + await fs.rm(`./test-exercise-repos/${repoName}`, { recursive: true, force: true }); + }src/test/playwright/e2e/exam/ExamParticipation.spec.ts (1)
261-261: Consider using '@fast' tag for Exam announcementsWhile the '@slow' tag works, these tests might be better suited for the '@fast' tag because:
- They test real-time communication features
- Don't involve submission results or complex UI workflows
- Could potentially run faster in parallel
- test.describe('Exam announcements', { tag: '@slow' }, () => { + test.describe('Exam announcements', { tag: '@fast' }, () => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (15)
src/test/playwright/e2e/exam/ExamAssessment.spec.ts(6 hunks)src/test/playwright/e2e/exam/ExamParticipation.spec.ts(5 hunks)src/test/playwright/e2e/exam/ExamResults.spec.ts(2 hunks)src/test/playwright/e2e/exam/ExamTestRun.spec.ts(1 hunks)src/test/playwright/e2e/exam/test-exam/TestExamCreationDeletion.spec.ts(1 hunks)src/test/playwright/e2e/exam/test-exam/TestExamManagement.spec.ts(1 hunks)src/test/playwright/e2e/exercise/ExerciseImport.spec.ts(4 hunks)src/test/playwright/e2e/exercise/modeling/ModelingExerciseParticipation.spec.ts(1 hunks)src/test/playwright/e2e/exercise/programming/ProgrammingExerciseAssessment.spec.ts(1 hunks)src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts(1 hunks)src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts(1 hunks)src/test/playwright/e2e/exercise/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts(1 hunks)src/test/playwright/e2e/exercise/quiz-exercise/QuizExerciseAssessment.spec.ts(1 hunks)src/test/playwright/package.json(1 hunks)src/test/playwright/playwright.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts
🧰 Additional context used
🪛 Biome
src/test/playwright/e2e/exam/ExamResults.spec.ts
[error] 146-154: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 160-160: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (25)
src/test/playwright/package.json (2)
20-20: LGTM! Well-structured sequential execution configuration
The script correctly enforces sequential execution with a single worker, which is essential for tests that must run in order (e.g., programming exercise submissions).
18-18: Consider the implications of --continue-on-error flag
While the flag ensures all tests are executed, it might mask critical failures in the parallel tests before running sequential tests. Consider adding a post-execution validation step to ensure no critical tests failed.
src/test/playwright/e2e/exercise/modeling/ModelingExerciseParticipation.spec.ts (1)
7-7: LGTM! The @fast tag is appropriate for this test suite.
The change from @slow to @fast aligns with the PR objectives since this test suite:
- Doesn't trigger programming exercise submissions
- Doesn't assert submission results
- Operates in isolation with its own course/exercise
- Can safely run in parallel with other tests
src/test/playwright/e2e/exercise/quiz-exercise/QuizExerciseAssessment.spec.ts (1)
Line range hint 8-53: Test implementation looks good!
The test implementation follows good practices:
- Proper setup/teardown using beforeEach/afterEach hooks
- Focused test cases with clear assertions
- Efficient use of API requests for operations
- Good flaky test handling with retry configuration
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts (1)
9-9: LGTM! Tag change aligns with PR objectives.
The change from @slow to @sequential is appropriate for this test suite because:
- It involves programming exercise submission and result assertion
- It has complex setup/teardown with course creation/deletion
- Sequential execution is necessary to maintain test stability
src/test/playwright/e2e/exam/test-exam/TestExamCreationDeletion.spec.ts (1)
26-26: LGTM: Tag change aligns with PR objectives
The change from '@slow' to '@fast' is appropriate as this test suite doesn't involve programming exercise submissions or assert submission results.
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseAssessment.spec.ts (1)
20-20: LGTM! Appropriate tag for sequential test execution.
The change from @slow to @sequential is correct as this test must run sequentially due to:
- Programming exercise submission verification
- Time-sensitive operations with due dates
- Complex state changes through multiple user roles
src/test/playwright/e2e/exam/test-exam/TestExamManagement.spec.ts (1)
14-14: Verify parallel execution safety for programming exercise tests.
The change from @slow to @fast tag aligns with the PR objectives since these tests don't assert programming exercise submissions. However, the programming exercise creation test might be resource-intensive.
Let's verify if there are any resource conflicts when running in parallel:
✅ Verification successful
Based on the search results, I can now generate the final response:
The tag change from @slow to @fast is appropriate for this test suite.
The codebase search reveals that programming exercise creation tests are well-isolated across different test files, and the test suite in question (TestExamManagement.spec.ts) only contains basic UI interactions for adding a programming exercise to an exam. The actual resource-intensive operations (like repository creation) are mocked or handled by the test fixtures, making parallel execution safe.
Key findings:
- The programming exercise creation test in this file only handles UI interactions and form submissions
- Resource-intensive operations are abstracted through fixtures (
programmingExerciseCreation) - Other test files containing programming exercise tests are properly tagged and isolated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar programming exercise creation tests that might run in parallel
ast-grep --pattern 'test($_, async ({ $$ }) => {
$$$
programmingExerciseCreation
$$$
})'
Length of output: 220
Script:
#!/bin/bash
# Let's try a different approach to search for programming exercise creation patterns
rg -l "programmingExercise" --type ts
# Then look for test files containing programming exercise related tests
rg "test\(.*programming.*exercise.*" --type ts -A 5
Length of output: 28273
src/test/playwright/e2e/exam/ExamTestRun.spec.ts (1)
Line range hint 20-45: Well-structured test setup and organization
The test suite follows best practices with:
- Clear separation of setup and teardown
- Proper async/await usage
- Well-organized test groups
- Focused assertions
src/test/playwright/e2e/exercise/ExerciseImport.spec.ts (4)
20-20: LGTM! Appropriate tag removal
The removal of the @slow tag from the describe block is correct since individual tests now have specific tags for better categorization.
42-42: LGTM! Appropriate use of @fast tag
The text exercise import test is correctly tagged as @fast since it doesn't involve programming exercise submissions or complex result assertions.
72-72: LGTM! Appropriate use of @fast tag
The quiz exercise import test is correctly tagged as @fast since it doesn't involve programming exercise submissions or complex result assertions.
126-150: LGTM! Appropriate use of @Sequential tag
The programming exercise import test is correctly tagged as @sequential since it involves programming exercise submission and result verification, which aligns with the PR's objectives for test categorization.
src/test/playwright/e2e/exam/ExamResults.spec.ts (1)
129-132: LGTM: Test categorization aligns with PR objectives
The conditional tagging implementation correctly categorizes programming exercise tests as @sequential and others as @slow, which aligns with the PR's goal of improving test execution speed while maintaining stability for programming exercise tests.
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts (1)
23-23: LGTM: Tag change aligns with test execution strategy
The change from @slow to @sequential correctly categorizes these tests as requiring sequential execution, which is appropriate given they involve programming exercise submissions and build result verification.
src/test/playwright/e2e/exam/ExamParticipation.spec.ts (5)
20-20: LGTM: Removal of '@slow' tag from main describe block
The removal of the '@slow' tag from the main describe block is appropriate since individual test blocks now have specific tags for better granularity in test execution.
46-46: LGTM: '@slow' tag for Early Hand-in tests
The '@slow' tag is correctly applied here as these tests:
- Don't assert submission results
- Focus on UI navigation and submission workflow
- Can safely run in parallel with other tests
120-120: LGTM: '@slow' tag for Early hand-in with continue tests
The '@slow' tag is appropriate as these tests:
- Focus on UI interactions and page reloads
- Don't involve critical submission assertions
- Can run in parallel with other tests
224-224: LGTM: '@Sequential' tag for Normal Hand-in tests
The '@Sequential' tag is correctly applied as these tests:
- Assert actual submission results
- Use time-sensitive operations (30-second window)
- Need to run sequentially to maintain stability
Line range hint 224-260: Verify timeout settings for time-sensitive tests
The Normal Hand-in tests use a 30-second window for exam completion. Ensure this timing is sufficient when running sequentially.
✅ Verification successful
Let me analyze the test timeouts and generate additional verification scripts to check the test configuration and sequential test execution settings.
30-second exam window is sufficient for sequential tests
The verification shows that:
- The test is properly tagged with
@sequential, ensuring it runs in isolation - Sequential tests have a generous timeout of 180 seconds (3 minutes) as configured in
playwright.config.ts - The 30-second exam window is only used for the actual exam submission timing, not the test execution timing
- Other exam-related tests use similar short durations (1-2 minutes) for exam windows successfully
The timing is appropriate because:
- Sequential tests run one at a time, eliminating concurrency issues
- The test performs simple text exercise submission
- The configured test timeout (180s) provides ample buffer for potential delays
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for timeout configurations in test setup
rg -A 2 "setTimeout|timeout:" src/test/playwright/
Length of output: 6427
Script:
#!/bin/bash
# Search for sequential test configurations and related timing settings
rg -A 2 "@sequential|test.describe.*sequential" src/test/playwright/
# Search for exam duration settings in the test files
rg -A 2 "endDate.*dayjs" src/test/playwright/
Length of output: 9635
src/test/playwright/e2e/exam/ExamAssessment.spec.ts (5)
56-57: LGTM: Serial mode configuration ensures correct test execution order.
The addition of test.describe.configure({ mode: 'serial' }) ensures that all nested tests run sequentially, which is crucial for maintaining the integrity of the exam assessment flow.
62-62: LGTM: Appropriate test categorization with @Sequential tag.
The @sequential tag correctly identifies this test block as requiring sequential execution due to its programming exercise submission assertions, aligning with the PR's goal of improving test organization.
157-157: LGTM: Appropriate test categorization with @slow tag.
The @slow tag is correctly applied as this test involves automatic assessment and timing-based operations but doesn't directly assert submission results.
187-187: LGTM: Appropriate test categorization with @fast tag.
The @fast tag is correctly applied as these tests can run independently and don't involve complex submission assertions.
226-226: LGTM: Appropriate test categorization with @Sequential tag.
The @sequential tag is correctly applied as this test requires ordered execution of multiple setup steps and involves multiple student submissions and assessments.
src/test/playwright/e2e/exercise/quiz-exercise/QuizExerciseAssessment.spec.ts
Show resolved
Hide resolved
Checklist
General
Client
Motivation and Context
Currently, execution of Playwright tests takes a long time due to a lot of tests running sequentially.
Description
This PR separates sequentially running tests that do not trigger any programming exercise submissions or do not assert the results of submissions into another category. These tests will now run in parallel to
@fasttests, but with larger timeout. These tests are now tagged with@slowannotation. All tests that assert the programming exercise submission results are now tagged as@sequentialto make the purpose more clear, and run sequentially to ensure stability.Steps for Testing
Steps for running the tests locally
npm install && npm run playwright:setupnpm run playwright:test./test-reports/results.xmlTestserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores