Hotove testy z kurzu automatizace#67
Open
magdiroubalova-eng wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR expands the test automation framework with a new order assertion helper, integrates it into the assertion facade, and adds comprehensive end-to-end UI tests covering authentication, navigation, ordering, application management, and password changes across multiple user scenarios. ChangesUI Test Automation Suite Expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/test/java/cz/czechitas/automation/MyFirstTest.java (2)
32-36: ⚡ Quick winConsider adding an assertion to verify ICO was set.
The test fills the ICO field but doesn't verify the value. Consider adding
asserter.orderSection.checkIcoFieldContainsValue("22834958")to make the test more robust, similar toorderWithICOAndSchoolInNatureTest()at line 67.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/cz/czechitas/automation/MyFirstTest.java` around lines 32 - 36, The test fillICOTest fills the ICO but lacks verification; after calling browser.orderSection.insertICO("22834958") add an assertion call to verify the field value using asserter.orderSection.checkIcoFieldContainsValue("22834958") (mirror the approach used in orderWithICOAndSchoolInNatureTest); ensure the call is placed inside the fillICOTest method immediately after browser.orderSection.insertICO to validate the ICO was set.
7-12: ⚡ Quick winConsider extracting credentials to constants.
The
login()helper uses hardcoded credentials that are also duplicated increateApplicationTest()(lines 40-41). Consider extracting user credentials to private static final fields for better maintainability.♻️ Suggested refactor
+ private static final String USER1_EMAIL = "manka.rumova@yahoo.com"; + private static final String USER1_PASSWORD = "Hovinko123"; + private static final String USER2_EMAIL = "peky@email.cz"; + private static final String USER2_PASSWORD = "Hovinko123"; + private void login() { browser.loginSection.clickLoginMenuLink(); - browser.loginSection.insertEmail("manka.rumova@yahoo.com"); - browser.loginSection.insertPassword("Hovinko123"); + browser.loginSection.insertEmail(USER1_EMAIL); + browser.loginSection.insertPassword(USER1_PASSWORD); browser.loginSection.clickLoginButton(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/cz/czechitas/automation/MyFirstTest.java` around lines 7 - 12, Extract the hardcoded credentials used in login() and createApplicationTest() into private static final fields (e.g., EMAIL and PASSWORD) at the class level; update the login() method (browser.loginSection.insertEmail and insertPassword) and any other places (createApplicationTest) to reference these constants instead of literal strings to avoid duplication and improve maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/java/cz/czechitas/automation/ExampleTest.java`:
- Around line 38-41: Remove the duplicate test method
navigateToParentInstructionsAndFormsTest found in class ExampleTest and keep the
single authoritative copy in MyFirstTest; locate the method
navigateToParentInstructionsAndFormsTest in
src/test/java/cz/czechitas/automation/ExampleTest.java and delete that method
(the one calling browser.headerMenu.goToInstructionsAndFormsForParentSection()),
then run tests to ensure no other references break and remove any now-unused
imports or setup in ExampleTest if they become unnecessary.
In `@src/test/java/cz/czechitas/automation/MyFirstTest.java`:
- Line 173: The test uses a 9-second hard wait via browser.waitFor(9) which is
slow and flaky; replace that call in MyFirstTest (the location using
browser.waitFor(9)) with an explicit wait that polls until the popup element is
gone or stale (use the test framework's invisibility/staleness helper, e.g.
browser.waitForElementToBeInvisible("<popup-selector>", timeout) or
WebDriverWait
until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector("<popup-selector>")))
), ensure you pick the correct popup selector and set a reasonable timeout (e.g.
10s) and remove the hard-coded wait.
- Around line 155-192: The changePasswordTest currently changes the account
password but may fail before restoring it; wrap the test flow so the password
reset always runs (e.g., put the steps that set the new password and verify
login inside try and perform the restore in a finally block), or move the
restore logic into an `@AfterEach` method that uses the same variables
(puvodniHeslo, noveHeslo) and calls the same page actions
(browser.profileSection.goToProfilePage(), insertPassword(),
insertPasswordVerification(), clickChangeButton()) and login/logout helpers
(browser.loginSection.logout(), clickLoginMenuLink(), insertEmail(),
insertPassword(), clickLoginButton()) to ensure the account is reset even if
asserter.checkIsLoggedIn() or later steps fail.
- Line 125: Replace the hard sleep call browser.waitFor(3) in MyFirstTest with
an explicit wait for the specific condition or element you need, e.g. remove
browser.waitFor(3) and use a WebDriverWait / ExpectedConditions (or the
project's browser.waitUntil equivalent) to wait for the target element/state (by
its locator or a helper method that returns a boolean) to be
visible/clickable/present before proceeding; this keeps the test fast and
robust.
- Around line 38-46: createApplicationTest() duplicates the login steps that are
already implemented in the login() helper (defined near lines 7-12); remove the
explicit calls to browser.loginSection.insertEmail / insertPassword /
clickLoginMenuLink / clickLoginButton in createApplicationTest() and replace
them with a single call to login(), then keep the subsequent applicationSection
calls (clickCreateNewApplicationButton, selectProgrammingSection,
clickCreatePythonApplicationButton) unchanged so the test reuses the helper and
avoids duplication.
---
Nitpick comments:
In `@src/test/java/cz/czechitas/automation/MyFirstTest.java`:
- Around line 32-36: The test fillICOTest fills the ICO but lacks verification;
after calling browser.orderSection.insertICO("22834958") add an assertion call
to verify the field value using
asserter.orderSection.checkIcoFieldContainsValue("22834958") (mirror the
approach used in orderWithICOAndSchoolInNatureTest); ensure the call is placed
inside the fillICOTest method immediately after browser.orderSection.insertICO
to validate the ICO was set.
- Around line 7-12: Extract the hardcoded credentials used in login() and
createApplicationTest() into private static final fields (e.g., EMAIL and
PASSWORD) at the class level; update the login() method
(browser.loginSection.insertEmail and insertPassword) and any other places
(createApplicationTest) to reference these constants instead of literal strings
to avoid duplication and improve maintainability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0055c9b7-6ce5-406b-bac4-a052c36f97a8
📒 Files selected for processing (4)
src/test/java/cz/czechitas/automation/ExampleTest.javasrc/test/java/cz/czechitas/automation/MyFirstTest.javasrc/test/java/cz/czechitas/automation/assertion/AssertionFacade.javasrc/test/java/cz/czechitas/automation/assertion/OrderAssertion.java
Comment on lines
+38
to
+41
| @Test | ||
| void navigateToParentInstructionsAndFormsTest() { | ||
| browser.headerMenu.goToInstructionsAndFormsForParentSection(); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all instances of navigateToParentInstructionsAndFormsTest across the test suite
# Search for the test method definition
rg -n "void navigateToParentInstructionsAndFormsTest\(\)" --type=javaRepository: czechitas/java-test-automation
Length of output: 298
Remove the duplicated test navigateToParentInstructionsAndFormsTest
navigateToParentInstructionsAndFormsTest exists in both src/test/java/cz/czechitas/automation/MyFirstTest.java (line 14) and src/test/java/cz/czechitas/automation/ExampleTest.java (line 39). Keep a single copy to avoid maintenance overhead and confusion, and delete the duplicate from one class.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/cz/czechitas/automation/ExampleTest.java` around lines 38 - 41,
Remove the duplicate test method navigateToParentInstructionsAndFormsTest found
in class ExampleTest and keep the single authoritative copy in MyFirstTest;
locate the method navigateToParentInstructionsAndFormsTest in
src/test/java/cz/czechitas/automation/ExampleTest.java and delete that method
(the one calling browser.headerMenu.goToInstructionsAndFormsForParentSection()),
then run tests to ensure no other references break and remove any now-unused
imports or setup in ExampleTest if they become unnecessary.
Comment on lines
+38
to
+46
| void createApplicationTest() { | ||
| browser.loginSection.clickLoginMenuLink(); | ||
| browser.loginSection.insertEmail("manka.rumova@yahoo.com"); | ||
| browser.loginSection.insertPassword("Hovinko123"); | ||
| browser.loginSection.clickLoginButton(); | ||
| browser.applicationSection.clickCreateNewApplicationButton(); | ||
| browser.applicationSection.selectProgrammingSection(); | ||
| browser.applicationSection.clickCreatePythonApplicationButton(); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the login() helper to avoid code duplication.
Lines 39-42 duplicate the login logic that's already extracted in the login() helper method (lines 7-12). Using the helper improves maintainability.
♻️ Suggested refactor
`@Test`
void createApplicationTest() {
- browser.loginSection.clickLoginMenuLink();
- browser.loginSection.insertEmail("manka.rumova@yahoo.com");
- browser.loginSection.insertPassword("Hovinko123");
- browser.loginSection.clickLoginButton();
+ login();
browser.applicationSection.clickCreateNewApplicationButton();
browser.applicationSection.selectProgrammingSection();
browser.applicationSection.clickCreatePythonApplicationButton();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void createApplicationTest() { | |
| browser.loginSection.clickLoginMenuLink(); | |
| browser.loginSection.insertEmail("manka.rumova@yahoo.com"); | |
| browser.loginSection.insertPassword("Hovinko123"); | |
| browser.loginSection.clickLoginButton(); | |
| browser.applicationSection.clickCreateNewApplicationButton(); | |
| browser.applicationSection.selectProgrammingSection(); | |
| browser.applicationSection.clickCreatePythonApplicationButton(); | |
| } | |
| void createApplicationTest() { | |
| login(); | |
| browser.applicationSection.clickCreateNewApplicationButton(); | |
| browser.applicationSection.selectProgrammingSection(); | |
| browser.applicationSection.clickCreatePythonApplicationButton(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/cz/czechitas/automation/MyFirstTest.java` around lines 38 - 46,
createApplicationTest() duplicates the login steps that are already implemented
in the login() helper (defined near lines 7-12); remove the explicit calls to
browser.loginSection.insertEmail / insertPassword / clickLoginMenuLink /
clickLoginButton in createApplicationTest() and replace them with a single call
to login(), then keep the subsequent applicationSection calls
(clickCreateNewApplicationButton, selectProgrammingSection,
clickCreatePythonApplicationButton) unchanged so the test reuses the helper and
avoids duplication.
| browser.applicationSection.clickCreateNewApplicationButton(); | ||
| browser.applicationSection.selectProgrammingSection(); | ||
| browser.applicationSection.clickCreatePythonApplicationButton(); | ||
| browser.waitFor(3); |
Contributor
There was a problem hiding this comment.
Replace hard wait with explicit wait.
The 3-second waitFor(3) is a hard-coded sleep that makes the test slower and potentially flaky. Consider using an explicit wait for the specific condition you're waiting for (e.g., waiting for a specific element or state to be ready).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/cz/czechitas/automation/MyFirstTest.java` at line 125, Replace
the hard sleep call browser.waitFor(3) in MyFirstTest with an explicit wait for
the specific condition or element you need, e.g. remove browser.waitFor(3) and
use a WebDriverWait / ExpectedConditions (or the project's browser.waitUntil
equivalent) to wait for the target element/state (by its locator or a helper
method that returns a boolean) to be visible/clickable/present before
proceeding; this keeps the test fast and robust.
Comment on lines
+155
to
+192
| @Test | ||
| void changePasswordTest() { | ||
| var noveHeslo = "NoveHeslo123"; | ||
| var puvodniHeslo = "Hovinko123"; | ||
|
|
||
| // Prihlaseni | ||
| browser.loginSection.clickLoginMenuLink(); | ||
| browser.loginSection.insertEmail("peky@email.cz"); | ||
| browser.loginSection.insertPassword(puvodniHeslo); | ||
| browser.loginSection.clickLoginButton(); | ||
|
|
||
| // Zmena hesla | ||
| browser.profileSection.goToProfilePage(); | ||
| browser.profileSection.insertPassword(noveHeslo); | ||
| browser.profileSection.insertPasswordVerification(noveHeslo); | ||
| browser.profileSection.clickChangeButton(); | ||
|
|
||
| // Prodleva nez zmizi pop up | ||
| browser.waitFor(9); | ||
|
|
||
| // Odhlaseni | ||
| browser.loginSection.logout(); | ||
|
|
||
| // Prihlaseni s novym heslem | ||
| browser.loginSection.clickLoginMenuLink(); | ||
| browser.loginSection.insertEmail("peky@email.cz"); | ||
| browser.loginSection.insertPassword(noveHeslo); | ||
| browser.loginSection.clickLoginButton(); | ||
|
|
||
| // Overeni prihlaseni | ||
| asserter.checkIsLoggedIn(); | ||
|
|
||
| // Zmena hesla zpet na puvodni | ||
| browser.profileSection.goToProfilePage(); | ||
| browser.profileSection.insertPassword(puvodniHeslo); | ||
| browser.profileSection.insertPasswordVerification(puvodniHeslo); | ||
| browser.profileSection.clickChangeButton(); | ||
| } |
Contributor
There was a problem hiding this comment.
Password change test lacks proper cleanup on failure.
If the test fails after changing the password (line 170) but before changing it back (line 191), the user account peky@email.cz will be left with the new password, potentially breaking subsequent test runs. Consider using a try-finally block or a test lifecycle hook (@AfterEach) to ensure the password is always reset, or use a dedicated test user account that can be reset independently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/cz/czechitas/automation/MyFirstTest.java` around lines 155 -
192, The changePasswordTest currently changes the account password but may fail
before restoring it; wrap the test flow so the password reset always runs (e.g.,
put the steps that set the new password and verify login inside try and perform
the restore in a finally block), or move the restore logic into an `@AfterEach`
method that uses the same variables (puvodniHeslo, noveHeslo) and calls the same
page actions (browser.profileSection.goToProfilePage(), insertPassword(),
insertPasswordVerification(), clickChangeButton()) and login/logout helpers
(browser.loginSection.logout(), clickLoginMenuLink(), insertEmail(),
insertPassword(), clickLoginButton()) to ensure the account is reset even if
asserter.checkIsLoggedIn() or later steps fail.
| browser.profileSection.clickChangeButton(); | ||
|
|
||
| // Prodleva nez zmizi pop up | ||
| browser.waitFor(9); |
Contributor
There was a problem hiding this comment.
Replace 9-second hard wait with explicit wait for popup dismissal.
The 9-second hard-coded sleep significantly slows down the test and is unreliable. The popup might disappear sooner or take longer. Use an explicit wait that polls for the popup element to become invisible or stale.
🔧 Suggested improvement
Replace the hard wait with an explicit wait that polls for the popup to disappear. For example (assuming the popup has a predictable selector):
// Instead of: browser.waitFor(9);
// Use explicit wait for popup to disappear, e.g.:
// browser.waitForElementToBeInvisible(".popup-selector", 10);Check if your test framework provides a utility for waiting on element invisibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/java/cz/czechitas/automation/MyFirstTest.java` at line 173, The test
uses a 9-second hard wait via browser.waitFor(9) which is slow and flaky;
replace that call in MyFirstTest (the location using browser.waitFor(9)) with an
explicit wait that polls until the popup element is gone or stale (use the test
framework's invisibility/staleness helper, e.g.
browser.waitForElementToBeInvisible("<popup-selector>", timeout) or
WebDriverWait
until(ExpectedConditions.invisibilityOfElementLocated(By.cssSelector("<popup-selector>")))
), ensure you pick the correct popup selector and set a reasonable timeout (e.g.
10s) and remove the hard-coded wait.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit