-
Notifications
You must be signed in to change notification settings - Fork 276
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 programming exercise team participation
#8559
Development
: Add e2e playwright tests for programming exercise team participation
#8559
Conversation
…rcise submission API request
…eam-participation
WalkthroughThe recent updates focus on refining the Playwright testing framework for programming exercises. These changes include enhancing test setup and execution, improving element targeting for better interaction with UI components, introducing new methods and page objects, adding constants for new features, and updating API requests to support programming exercise configurations effectively. Changes
Possibly related issues
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: 8
Out of diff range and nitpick comments (5)
src/test/playwright/support/pageobjects/exercises/programming/RepositoryPage.ts (1)
17-18
: Clarify the reason for reversing the commits array.It would be helpful to add a comment explaining why the commits array is reversed, as this might not be immediately obvious to other developers or maintainers.
src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseOverviewPage.ts (1)
44-50
: Add documentation for the methodsgetCloneRepositoryButton
andgetExerciseDetails
.It would be beneficial to add JSDoc comments for the methods
getCloneRepositoryButton
andgetExerciseDetails
to improve code readability and maintainability.src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts (3)
8-8
: Consider adding a brief comment explaining the purpose of the imported entities for better code readability.
Line range hint
54-77
: The conditional logic to skip C tests based on the database type is a good practice to ensure compatibility. However, consider externalizing such environment-specific configurations to a separate configuration file or environment variables to enhance maintainability and scalability.
261-261
: Add a detailed comment explaining the purpose and usage of thepushGitSubmissionFiles
function to improve code readability and maintainability.
...laywright/support/pageobjects/exercises/programming/ProgrammingExerciseParticipationsPage.ts
Show resolved
Hide resolved
...laywright/support/pageobjects/exercises/programming/ProgrammingExerciseParticipationsPage.ts
Outdated
Show resolved
Hide resolved
...laywright/support/pageobjects/exercises/programming/ProgrammingExerciseParticipationsPage.ts
Outdated
Show resolved
Hide resolved
...laywright/support/pageobjects/exercises/programming/ProgrammingExerciseParticipationsPage.ts
Outdated
Show resolved
Hide resolved
src/test/playwright/support/pageobjects/exercises/programming/RepositoryPage.ts
Outdated
Show resolved
Hide resolved
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: 0
Out of diff range and nitpick comments (3)
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts (3)
8-8
: Consider adding a brief comment explaining the purpose of the imported entities for better code readability.
Line range hint
54-77
: The conditional logic to skip C tests based on the database type (Postgres
) is a good practice to ensure compatibility. However, consider externalizing such environment-specific configurations to a separate configuration file or utility function to enhance modularity and maintainability.
262-262
: The functionpushGitSubmissionFiles
is crucial for setting up the test environment. Consider adding more detailed comments explaining each step of the process, which would be beneficial for new contributors or during debugging sessions.
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
Out of diff range and nitpick comments (3)
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts (3)
8-8
: Consider adding a brief comment describing the purpose of the imported entities for better code readability.
[!TIP]
Codebase VerificationThe verification process has revealed that the
PLAYWRIGHT_DB_TYPE
environment variable is set differently across various Docker configuration files used for CI environments. Specifically, it is set to 'MySQL' in the configurations for MySQL setups and to 'Postgres' in the configuration for Postgres setup. This indicates that the environment variable is not consistently set to 'Postgres' across all environments, which aligns with the conditional logic in the test script to skip C tests when using Postgres.Given this information, the review comment is accurate in highlighting the potential for unintentional skipping of C tests due to the variability in the setting of
PLAYWRIGHT_DB_TYPE
across different environments.Final Response:
The verification has confirmed that the
PLAYWRIGHT_DB_TYPE
environment variable is set differently in various CI configurations:
docker/playwright-E2E-tests-mysql-localci.yml
anddocker/playwright-E2E-tests-mysql.yml
set it to 'MySQL'.docker/playwright-E2E-tests-postgres.yml
sets it to 'Postgres'.This variability can indeed lead to the unintentional skipping of C tests in environments not using Postgres, as the test script checks for this variable to decide whether to skip. It is recommended to ensure consistency in the environment settings or adjust the test script to handle different environments appropriately.
- File locations:
docker/playwright-E2E-tests-mysql-localci.yml
docker/playwright-E2E-tests-mysql.yml
docker/playwright-E2E-tests-postgres.yml
Analysis chain
Line range hint
54-77
: Ensure that the environment variablePLAYWRIGHT_DB_TYPE
is consistently set across different environments to avoid skipping C tests unintentionally.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the PLAYWRIGHT_DB_TYPE environment variable is set to 'Postgres' in all CI environments. # Test: Search for environment variable settings in CI configuration files. Expect: 'Postgres' should be set for PLAYWRIGHT_DB_TYPE. rg --type yaml --type json 'PLAYWRIGHT_DB_TYPE: Postgres'Length of output: 59
Script:
#!/bin/bash # Description: Broaden the search to find any instances of the PLAYWRIGHT_DB_TYPE environment variable being set, regardless of its value. # Search across all file types for the environment variable setting. rg 'PLAYWRIGHT_DB_TYPE' --files-with-matchesLength of output: 255
Script:
#!/bin/bash # Description: Extract the lines setting the PLAYWRIGHT_DB_TYPE environment variable in identified Docker configuration files. # Extract relevant lines from each configuration file. cat docker/playwright-E2E-tests-mysql-localci.yml cat docker/playwright-E2E-tests-postgres.yml cat docker/playwright-E2E-tests-mysql.yml grep 'PLAYWRIGHT_DB_TYPE' docker/playwright-E2E-tests-mysql-localci.yml grep 'PLAYWRIGHT_DB_TYPE' docker/playwright-E2E-tests-postgres.yml grep 'PLAYWRIGHT_DB_TYPE' docker/playwright-E2E-tests-mysql.ymlLength of output: 6366
262-262
: The functionpushGitSubmissionFiles
lacks documentation for thedeleteFiles
parameter. Adding a comment explaining its purpose would enhance code clarity.
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts
Outdated
Show resolved
Hide resolved
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts
Show resolved
Hide resolved
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.
code lgtm!
Test passed on CI: Bamboo tests
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 have left some comments where I think that changes might be needed.
Besides the comments I think it would be good if the comments of coderabbit would be either resolved, or commented on if there is something unclear about them.
Not all suggestions might be useful, if a comment doesn't make sense it would still be beneficial to resolve that conversation.
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts
Outdated
Show resolved
Hide resolved
src/test/playwright/support/pageobjects/exercises/programming/RepositoryPage.ts
Outdated
Show resolved
Hide resolved
src/test/playwright/support/pageobjects/exercises/programming/RepositoryPage.ts
Outdated
Show resolved
Hide resolved
src/test/playwright/e2e/exercise/programming/ProgrammingExerciseParticipation.spec.ts
Show resolved
Hide resolved
...laywright/support/pageobjects/exercises/programming/ProgrammingExerciseParticipationsPage.ts
Outdated
Show resolved
Hide resolved
…ensure builds have time to finish
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.
Demonstrated in testing session, works as expected.
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
…eam-participation
d75e67a
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.
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.
Reapprove after merge
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
…eam-participation
Development
: Add e2e tests for programming exercise team participationDevelopment
: Add e2e playwright tests for programming exercise team participation
Checklist
General
Client
Motivation and Context
We want to test programming exercise participations by teams.
Description
Adds E2E tests in Playwright about programming exercise team participations.
Steps for Testing
Steps for running the tests:
npm install && npm run playwright:setup && npx playwright test e2e/exercise/programming/ProgrammingExerciseParticipation.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
Summary by CodeRabbit
New Features
Enhancements
Documentation
Refactor