Skip to content

Review update #2

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.

Sign up for GitHub

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

Open
RomanNadia wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Review update #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2afc675
changes after review
RomanNadia Aug 23, 2024
0b4a40c
Merge branch 'main' of https://github.com/RomanNadia/ta_training-java…
RomanNadia Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 5 additions & 12 deletions src/main/java/com/saucedemo/driver/managment/DriverManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ private DriverManager() {}
public static WebDriver getDriver(Browser browser) {
if (driver == null) {
switch (browser) {
case CHROME:
WebDriverManager.chromedriver().setup();
driver = new ChromeDriver();
break;
case FIREBOX:
WebDriverManager.firefoxdriver().setup();
driver = new FirefoxDriver();
Expand All @@ -26,17 +22,14 @@ public static WebDriver getDriver(Browser browser) {
WebDriverManager.edgedriver().setup();
driver = new EdgeDriver();
break;
// TODO make default case
case CHROME:
default:
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added default case

WebDriverManager.chromedriver().setup();
driver = new ChromeDriver();
break;
}
}
return driver;
}

// TODO remove it if u don't use it
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused method

public static void quitDriver() {
if (driver != null) {
driver.quit();
driver = null;
}
}
}
8 changes: 8 additions & 0 deletions src/main/java/com/saucedemo/models/User.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,12 @@ public String getPassword() {
public void setPassword(String password) {
this.password = password;
}

Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added toString method for correct representation of user in logs

@Override
public String toString() {
return "User{" +
"username='" + username + '\'' +
", password='" + password + '\'' +
'}';
}
}
42 changes: 10 additions & 32 deletions src/main/java/com/saucedemo/pages/LoginPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,18 @@

import com.saucedemo.models.User;
import com.saucedemo.tests.LoginTest;
import org.openqa.selenium.TimeoutException;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
import org.openqa.selenium.support.PageFactory;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.time.Duration;
import java.util.List;

public class LoginPage {
// TODO don't forget to format your code(CTRL+ALT+L on windows) and remove unused imports(CTRL+ALT+O on windows)
private final WebDriver driver;
private static final Logger logger = LoggerFactory.getLogger(LoginTest.class);
private static final String LINK = "https://www.saucedemo.com/";

@FindBy(xpath = "//input[@data-test='username']")
private WebElement usernameArea;
Expand All @@ -29,7 +25,7 @@ public class LoginPage {
private WebElement loginButton;

@FindBy(xpath = "//*[@data-test='error']")
private List<WebElement> errorMassage; // TODO massage is great thing, but we need message :)
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed and change type to WebElement

private WebElement errorMessage;

public LoginPage(WebDriver driver) {
this.driver = driver;
Expand All @@ -38,7 +34,7 @@ public LoginPage(WebDriver driver) {


public LoginPage enterLoginFields(User user) {
logger.info("Entering login fields for user: {}", user); // TODO in logs u will have object reference, not the value of user
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added toString method in User class

logger.info("Entering login fields for user: {}", user);

enterUsername(user.getUsername());
enterPassword(user.getPassword());
Expand All @@ -47,16 +43,11 @@ public LoginPage enterLoginFields(User user) {


public LoginPage enterUsername(String username) {
// TODO remove commented-out code if it is not needed or make initialisation of WebDriverWait in the constructor
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented-out code

// WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
// wait.until(ExpectedConditions.visibilityOf(usernameArea));

if(username != null) {
usernameArea.sendKeys(username);
logger.debug("Entered username: {}", username);
} else {
String password = passwordArea.getAttribute("value");;
// TODO I'm not quite understand why do we need get password and refresh page if username is empty
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume page is doing chashing (or something like that), so whithout refresh input fields remember old values which was set to them even after using method clear, so in the button click moment the old values are send.

driver.navigate().refresh();
usernameArea.clear();
logger.debug("Cleared username field");
Expand All @@ -69,16 +60,11 @@ public LoginPage enterUsername(String username) {


public LoginPage enterPassword(String password) {
// TODO remove commented-out code if it is not needed or make initialisation of WebDriverWait in the constructor
// WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
// wait.until(ExpectedConditions.visibilityOf(passwordArea));

if(password != null) {
if(password != null) {
passwordArea.sendKeys(password);
logger.debug("Entered password: {}", password);
} else {
String username = usernameArea.getAttribute("value");
// TODO I'm not quite understand why do we need get username and refresh page if password is empty
driver.navigate().refresh();
passwordArea.clear();
logger.debug("Cleared password field");
Expand All @@ -91,32 +77,24 @@ public LoginPage enterPassword(String password) {


public LoginButtonClickResult clickLoginButton() {
// TODO remove commented-out code if it is not needed or make initialisation of WebDriverWait in the constructor
// WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
// wait.until(ExpectedConditions.visibilityOf(loginButton));

loginButton.click();
logger.info("Clicked login button");

if(driver.getCurrentUrl().equals("https://www.saucedemo.com/")) { // TODO to constants
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constant is added

if(driver.getCurrentUrl().equals(LINK)) {
return new LoginButtonClickResult(this);
} else {
return new LoginButtonClickResult(new ShopPage(driver));
}
}

// TODO is it necessary to make errorMessage as a List?
// TODO in each test u can get only one error message and then handle it or assert
// TODO it's not that locator and case as well where u collect a lot of errors,
// so it is better to use String for errorMessage

public String getErrorMassageText() {
if(!errorMassage.isEmpty()) {
String fullText = errorMassage.get(0).getText();
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errorMessage was made as WebElement. Also a check if element exists is remade

try {
String fullText = errorMessage.getText();
return fullText.substring(14);
} else {
} catch (TimeoutException e) {
return "";
}
}


}
17 changes: 7 additions & 10 deletions src/main/java/com/saucedemo/tests/LoginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@
import com.saucedemo.models.User;
import com.saucedemo.pages.LoginButtonClickResult;
import com.saucedemo.pages.LoginPage;
import java.util.stream.Stream;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.openqa.selenium.WebDriver;

import java.util.stream.Stream;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;

@TestInstance(TestInstance.Lifecycle.PER_METHOD)
public class LoginTest {
Expand Down Expand Up @@ -68,20 +69,17 @@ public void testLoginWithEmptyPassword(User user) {

@ParameterizedTest
@MethodSource("DataProviderForLogin")
public void testLoginWithCorrectInput() {
// TODO run this test and check logs, u testing 6 times that "standard_user" and "secret_sauce" are valid credentials
// TODO also u have this credentials in your DataProviderForLogin
User correctUser = new User("standard_user", "secret_sauce");
Copy link
Owner Author

@RomanNadia RomanNadia Aug 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using user from DataProviderForLogin method

public void testLoginWithCorrectInput(User user) {
LoginPage loginPage = new LoginPage(getDriver());
loginPage.enterLoginFields(correctUser);
loginPage.enterLoginFields(user);

LoginButtonClickResult loginButtonClickResult = loginPage.clickLoginButton();

assertThat(loginButtonClickResult.getShopPage(), notNullValue());
assertThat(loginButtonClickResult.getShopPage().getShopPageTitle(), equalTo("Swag Labs"));
}

// TODO u created great method for it in DriverManager class

@AfterEach
public void setDown() {
if (getDriver() != null) {
Expand All @@ -94,7 +92,6 @@ public void setDown() {
static Stream<Arguments> DataProviderForLogin() {
return Stream.of(
Arguments.of(new User("standard_user", "secret_sauce")),
Arguments.of(new User("locked_out_user", "secret_sauce")),
Arguments.of(new User("problem_user", "secret_sauce")),
Arguments.of(new User("performance_glitch_user", "secret_sauce")),
Arguments.of(new User("error_user", "secret_sauce")),
Expand Down