Skip to content
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

test: contributor page #1900

Merged
merged 1 commit into from
Sep 29, 2024
Merged

test: contributor page #1900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/main/webapp/WEB-INF/jsp/contributor/contributor.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
</c:choose>
</content:title>

<content:section cssId="contributorStoryBooksPage">
<content:section cssId="contributorPage">
<%@ include file="/WEB-INF/jsp/contributor/contributor-summarized.jsp" %>

<ul class="tabs tabs-transparent deep-purple lighten-1 z-depth-1" style="border-radius: 8px;">
Expand Down
2 changes: 1 addition & 1 deletion src/main/webapp/WEB-INF/jsp/contributor/list.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<c:forEach var="contributor" items="${contributors}">
<div class="col s6 m4 l3">
<div class="card contributor">
<a href="<spring:url value='/contributor/${contributor.id}' />">
<a class="contributorCardImageLink" href="<spring:url value='/contributor/${contributor.id}' />">
<div class="card-image">
<c:choose>
<c:when test="${not empty contributor.imageUrl}">
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/selenium/contributor/ContributorListPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
package selenium.contributor;

import java.util.List;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;

import selenium.util.ErrorHelper;

public class ContributorListPage {

private Logger logger = LogManager.getLogger();

private WebDriver driver;

public ContributorListPage(WebDriver driver) {
Expand All @@ -16,4 +23,13 @@ public ContributorListPage(WebDriver driver) {

ErrorHelper.verifyNoScriptOrMarkupError(driver);
}

public void pressRandomContributor() {
List<WebElement> contributorLinks = driver.findElements(By.className("contributorCardImageLink"));
logger.info("contributorLinks.size(): " + contributorLinks.size());
int randomIndex = (int) (Math.random() * contributorLinks.size());
logger.info("randomIndex: " + randomIndex);
WebElement contributorLink = contributorLinks.get(randomIndex);
contributorLink.click();
}
Comment on lines +27 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider handling edge cases and adding explicit waits.

The pressRandomContributor() method looks good overall, but there are a few points to consider:

  1. Handle the case where no contributor links are found to avoid potential IndexOutOfBoundsException.
  2. Add an explicit wait for the elements to be clickable to prevent race conditions.
  3. Consider using ThreadLocalRandom instead of Math.random() for better performance in concurrent scenarios.

Here's a suggested improvement:

import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import java.time.Duration;
import java.util.concurrent.ThreadLocalRandom;

public void pressRandomContributor() {
    WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
    List<WebElement> contributorLinks = wait.until(ExpectedConditions.presenceOfAllElementsLocatedBy(By.className("contributorCardImageLink")));
    
    logger.info("contributorLinks.size(): " + contributorLinks.size());
    
    if (contributorLinks.isEmpty()) {
        logger.warn("No contributor links found on the page.");
        return;
    }
    
    int randomIndex = ThreadLocalRandom.current().nextInt(contributorLinks.size());
    logger.info("randomIndex: " + randomIndex);
    
    WebElement contributorLink = contributorLinks.get(randomIndex);
    wait.until(ExpectedConditions.elementToBeClickable(contributorLink)).click();
}

This version includes error handling, explicit waits, and uses ThreadLocalRandom for better performance.

}
3 changes: 3 additions & 0 deletions src/test/java/selenium/contributor/ContributorListPageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,8 @@ public void testContributorListPage() {
logger.info("testContributorListPage");

ContributorListPage contributorListPage = new ContributorListPage(driver);
contributorListPage.pressRandomContributor();

ContributorPage contributorPage = new ContributorPage(driver);
Comment on lines +51 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Required methods are not implemented

The methods pressRandomContributor(), isPageLoaded(), and getContributorName() referenced in your test changes are not implemented in the ContributorListPage and ContributorPage classes. Please ensure these methods are defined to support the updated test functionality.

🔗 Analysis chain

Enhance test with assertions and error handling

The addition of pressRandomContributor() and the creation of a ContributorPage instance improve the test by simulating user interaction and page navigation. However, the test could be further improved:

  1. Add assertions to verify that the navigation to the contributor page was successful.
  2. Implement error handling to manage cases where no contributors are available.
  3. Consider verifying that the correct contributor page was loaded after the random selection.

Here's a suggested improvement:

ContributorListPage contributorListPage = new ContributorListPage(driver);
String selectedContributorName = contributorListPage.pressRandomContributor();
assertNotNull(selectedContributorName, "No contributor was selected");

ContributorPage contributorPage = new ContributorPage(driver);
assertTrue(contributorPage.isPageLoaded(), "Contributor page did not load");
assertEquals(selectedContributorName, contributorPage.getContributorName(), "Incorrect contributor page loaded");

This assumes that you modify pressRandomContributor() to return the name of the selected contributor, and add methods isPageLoaded() and getContributorName() to the ContributorPage class.

To ensure that the ContributorListPage and ContributorPage classes have the necessary methods, let's verify their implementations:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for the implementation of required methods in ContributorListPage and ContributorPage

echo "Checking ContributorListPage implementation:"
ast-grep --lang java --pattern $'class ContributorListPage {
  $$$
  String pressRandomContributor() {
    $$$
  }
  $$$
}'

echo "Checking ContributorPage implementation:"
ast-grep --lang java --pattern $'class ContributorPage {
  $$$
  boolean isPageLoaded() {
    $$$
  }
  $$$
  String getContributorName() {
    $$$
  }
  $$$
}'

Length of output: 476

}
}
18 changes: 18 additions & 0 deletions src/test/java/selenium/contributor/ContributorPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package selenium.contributor;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import selenium.util.ErrorHelper;

public class ContributorPage {

private WebDriver driver;

public ContributorPage(WebDriver driver) {
this.driver = driver;

driver.findElement(By.id("contributorPage"));

ErrorHelper.verifyNoScriptOrMarkupError(driver);
}
}
Comment on lines +7 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding methods for page interactions.

The ContributorPage class provides a good foundation, but it currently lacks methods for interacting with elements on the contributor page. To make this class more useful for testing, consider adding methods that represent actions a user might take on the contributor page.

Here are some suggestions for methods you might want to add:

public void enterContributorName(String name) {
    // Implementation to find and fill in a name field
}

public void selectContributorType(String type) {
    // Implementation to select a contributor type from a dropdown
}

public void submitContributorForm() {
    // Implementation to submit the contributor form
}

public boolean isSuccessMessageDisplayed() {
    // Implementation to check if a success message is displayed after form submission
}

These are just examples; the actual methods would depend on the specific functionality of your contributor page.

55 changes: 55 additions & 0 deletions src/test/java/selenium/contributor/ContributorPageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package selenium.contributor;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.chrome.ChromeDriver;
import org.openqa.selenium.chrome.ChromeOptions;

import selenium.util.DomainHelper;

public class ContributorPageTest {

private final Logger logger = LogManager.getLogger();

private WebDriver driver;

@BeforeEach
public void setUp() {
logger.info("setUp");

ChromeOptions chromeOptions = new ChromeOptions();

// Read "headless" property set on the command line:
// mvn clean verify -P regression-test-ui -D headless=true
String headlessSystemProperty = System.getProperty("headless");
logger.info("headlessSystemProperty: \"" + headlessSystemProperty + "\"");
if ("true".equals(headlessSystemProperty)) {
chromeOptions.addArguments("headless");
}

driver = new ChromeDriver(chromeOptions);

driver.get(DomainHelper.getBaseUrl() + "/contributor/list");
}

@AfterEach
public void tearDown() {
logger.info("tearDown");

driver.quit();
}

@Test
public void testContributorPage() {
logger.info("testContributorPage");

ContributorListPage contributorListPage = new ContributorListPage(driver);
contributorListPage.pressRandomContributor();

ContributorPage contributorPage = new ContributorPage(driver);
}
Comment on lines +46 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add assertions to verify expected behavior.

The test method structure is good, but it currently lacks assertions to verify the expected behavior after pressing a random contributor.

Consider adding assertions to validate the state of the ContributorPage after navigation. For example:

@Test
public void testContributorPage() {
    logger.info("testContributorPage");
    
    ContributorListPage contributorListPage = new ContributorListPage(driver);
    String selectedContributorName = contributorListPage.pressRandomContributor();

    ContributorPage contributorPage = new ContributorPage(driver);
    assertTrue(contributorPage.isLoaded(), "Contributor page should be loaded");
    assertEquals(selectedContributorName, contributorPage.getContributorName(), "Contributor name should match selected contributor");
    // Add more assertions to verify other elements on the ContributorPage
}

Make sure to implement the necessary methods in the ContributorListPage and ContributorPage classes to support these assertions.

}
Loading