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.

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

Improve UI test coverage #2048

Merged
merged 13 commits into from
Mar 4, 2025
2 changes: 1 addition & 1 deletion pom-dependency-tree.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ai.elimu:webapp:war:2.5.32-SNAPSHOT
ai.elimu:webapp:war:2.5.34-SNAPSHOT
+- com.github.elimu-ai:model:jar:model-2.0.83:compile
| \- com.google.code.gson:gson:jar:2.11.0:compile
| \- com.google.errorprone:error_prone_annotations:jar:2.27.0:compile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@
</div>

<div class="fixed-action-btn" style="bottom: 2em; right: 2em;">
<a href="<spring:url value='/content/letter-sound/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.letter.sound.correspondence" />"><i class="material-icons">add</i></a>
<a id="createButton" href="<spring:url value='/content/letter-sound/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.letter.sound.correspondence" />"><i class="material-icons">add</i></a>
</div>
</content:section>
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/content/letter/list.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
</td>
<td style="font-size: 2em;">
<a name="${letter.id}"></a>
<a href="<spring:url value='/content/letter/edit/${letter.id}' />">"<c:out value='${letter.text}' />"</a>
<a class="editLink" href="<spring:url value='/content/letter/edit/${letter.id}' />">"<c:out value='${letter.text}' />"</a>
</td>
<td>
<c:choose>
Expand Down Expand Up @@ -81,6 +81,6 @@
</div>

<div class="fixed-action-btn" style="bottom: 2em; right: 2em;">
<a href="<spring:url value='/content/letter/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.letter" />"><i class="material-icons">add</i></a>
<a id="createButton" href="<spring:url value='/content/letter/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.letter" />"><i class="material-icons">add</i></a>
</div>
</content:section>
8 changes: 4 additions & 4 deletions src/main/webapp/WEB-INF/jsp/content/main.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<span class="card-title"><i class="material-icons">text_fields</i> <fmt:message key="letters" /></span>
</div>
<div class="card-action">
<a href="<spring:url value='/content/letter/list' />"><fmt:message key="view.list" /> (${letterCount})</a>
<a id="letterListLink" href="<spring:url value='/content/letter/list' />"><fmt:message key="view.list" /> (${letterCount})</a>
</div>
</div>
</div>
Expand All @@ -25,7 +25,7 @@
<span class="card-title"><i class="material-icons">music_note</i> <fmt:message key="sounds" /></span>
</div>
<div class="card-action">
<a href="<spring:url value='/content/sound/list' />"><fmt:message key="view.list" /> (${soundCount})</a>
<a id="soundListLink" href="<spring:url value='/content/sound/list' />"><fmt:message key="view.list" /> (${soundCount})</a>
</div>
</div>
</div>
Expand All @@ -36,7 +36,7 @@
<span class="card-title"><i class="material-icons">emoji_symbols</i> <fmt:message key="letter.sounds" /></span>
</div>
<div class="card-action">
<a href="<spring:url value='/content/letter-sound/list' />"><fmt:message key="view.list" /> (${letterSoundCount})</a>
<a id="letterSoundListLink" href="<spring:url value='/content/letter-sound/list' />"><fmt:message key="view.list" /> (${letterSoundCount})</a>
<a href="<spring:url value='/content/letter-sound/peer-reviews' />"><fmt:message key="peer.review" /></a>
</div>
</div>
Expand All @@ -59,7 +59,7 @@
<span class="card-title"><i class="material-icons">looks_one</i> <fmt:message key="numbers" /></span>
</div>
<div class="card-action">
<a href="<spring:url value='/content/number/list' />"><fmt:message key="view.list" /> (${numberCount})</a>
<a id="numberListLink" href="<spring:url value='/content/number/list' />"><fmt:message key="view.list" /> (${numberCount})</a>
<a href="<spring:url value='/content/number/peer-reviews' />"><fmt:message key="peer.review" /></a>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/content/number/list.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<tr class="letter">
<td style="font-size: 2em;">
<a name="${number.id}"></a>
<a href="<spring:url value='/content/number/edit/${number.id}' />">${number.value}</a>
<a class="editLink" href="<spring:url value='/content/number/edit/${number.id}' />">${number.value}</a>
</td>
<td style="font-size: 2em;">
${number.symbol}
Expand Down Expand Up @@ -78,6 +78,6 @@
</div>

<div class="fixed-action-btn" style="bottom: 2em; right: 2em;">
<a href="<spring:url value='/content/number/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.number" />"><i class="material-icons">add</i></a>
<a id="createButton" href="<spring:url value='/content/number/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.number" />"><i class="material-icons">add</i></a>
</div>
</content:section>
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/content/sound/list.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
</td>
<td style="font-size: 2em;">
<a name="${sound.id}"></a>
<a href="<spring:url value='/content/sound/edit/${sound.id}' />">/${sound.valueIpa}/</a>
<a class="editLink" href="<spring:url value='/content/sound/edit/${sound.id}' />">/${sound.valueIpa}/</a>
</td>
<td>
${sound.valueSampa}
Expand Down Expand Up @@ -84,6 +84,6 @@
</div>

<div class="fixed-action-btn" style="bottom: 2em; right: 2em;">
<a href="<spring:url value='/content/sound/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.sound" />"><i class="material-icons">add</i></a>
<a id="createButton" href="<spring:url value='/content/sound/create' />" class="btn-floating btn-large tooltipped" data-position="left" data-delay="50" data-tooltip="<fmt:message key="add.sound" />"><i class="material-icons">add</i></a>
</div>
</content:section>
36 changes: 36 additions & 0 deletions src/test/java/selenium/content/MainContentPage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package selenium.content;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;

public class MainContentPage {

private WebDriver driver;

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

driver.findElement(By.id("mainContentPage"));
}

public void pressLetterListLink() {
WebElement link = driver.findElement(By.id("letterListLink"));
link.click();
}

public void pressSoundListLink() {
WebElement link = driver.findElement(By.id("soundListLink"));
link.click();
}

public void pressLetterSoundListLink() {
WebElement link = driver.findElement(By.id("letterSoundListLink"));
link.click();
}

public void pressNumberListLink() {
WebElement link = driver.findElement(By.id("numberListLink"));
link.click();
}
}
15 changes: 15 additions & 0 deletions src/test/java/selenium/content/letter/LetterCreatePage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package selenium.content.letter;

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

public class LetterCreatePage {

private WebDriver driver;

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

driver.findElement(By.id("letterCreatePage"));
}
}
15 changes: 15 additions & 0 deletions src/test/java/selenium/content/letter/LetterEditPage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package selenium.content.letter;

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

public class LetterEditPage {

private WebDriver driver;

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

driver.findElement(By.id("letterEditPage"));
}
}
Comment on lines +1 to +15
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 implementing a base page object to reduce duplication across page classes.

I notice a pattern of similar code structure across the different page objects. Consider creating a base page class to reduce duplication and improve maintainability.

Here's a suggested approach using a base page class:

package selenium.content.letter;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+import org.openqa.selenium.support.ui.WebDriverWait;
+import java.time.Duration;
+import selenium.content.BasePage;

-public class LetterEditPage {
+/**
+ * Page object representing the Letter Edit page.
+ */
+public class LetterEditPage extends BasePage {
-    
-    private WebDriver driver;
-
    public LetterEditPage(WebDriver driver) {
-        this.driver = driver;
-
-        driver.findElement(By.id("letterEditPage"));
+        super(driver, "letterEditPage");
    }
+    
+    // Add methods specific to letter editing functionality
}

And create a base page class:

package selenium.content;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.WebDriverWait;
import java.time.Duration;

/**
 * Base class for all page objects.
 * Handles common functionality like waiting for page elements.
 */
public abstract class BasePage {
    
    protected WebDriver driver;
    protected WebDriverWait wait;
    
    /**
     * Constructor for BasePage.
     * 
     * @param driver The WebDriver instance
     * @param pageIdentifier The ID of an element that uniquely identifies this page
     */
    public BasePage(WebDriver driver, String pageIdentifier) {
        this.driver = driver;
        this.wait = new WebDriverWait(driver, Duration.ofSeconds(10));
        
        // Wait for the page identifier to be present
        wait.until(ExpectedConditions.presenceOfElementLocated(By.id(pageIdentifier)));
    }
    
    /**
     * Waits for an element to be visible and returns it.
     * 
     * @param locator The By locator to find the element
     * @return The WebElement once it's visible
     */
    protected WebElement waitForVisibility(By locator) {
        return wait.until(ExpectedConditions.visibilityOfElementLocated(locator));
    }
    
    /**
     * Waits for an element to be clickable and returns it.
     * 
     * @param locator The By locator to find the element
     * @return The WebElement once it's clickable
     */
    protected WebElement waitForClickable(By locator) {
        return wait.until(ExpectedConditions.elementToBeClickable(locator));
    }
}

30 changes: 30 additions & 0 deletions src/test/java/selenium/content/letter/LetterListPage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package selenium.content.letter;

import java.util.List;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;

public class LetterListPage {

private WebDriver driver;

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

driver.findElement(By.id("letterListPage"));
}

public void pressRandomLetter() {
List<WebElement> links = driver.findElements(By.className("editLink"));
int randomIndex = (int) (Math.random() * links.size());
WebElement randomLink = links.get(randomIndex);
randomLink.click();
}

public void pressCreateButton() {
WebElement button = driver.findElement(By.id("createButton"));
button.click();
}
}
70 changes: 70 additions & 0 deletions src/test/java/selenium/content/letter/LetterTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package selenium.content.letter;

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 lombok.extern.slf4j.Slf4j;
import selenium.content.MainContentPage;
import selenium.util.DomainHelper;

@Slf4j
public class LetterTest {

private WebDriver driver;

@BeforeEach
public void setUp() {
log.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");
log.info("headlessSystemProperty: \"" + headlessSystemProperty + "\"");
if ("true".equals(headlessSystemProperty)) {
chromeOptions.addArguments("headless");
}

driver = new ChromeDriver(chromeOptions);

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

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

driver.quit();
}

@Test
public void testRandomLetterEditPage() {
log.info("testRandomLetterEditPage");

MainContentPage mainContentPage = new MainContentPage(driver);
mainContentPage.pressLetterListLink();

LetterListPage letterListPage = new LetterListPage(driver);
letterListPage.pressRandomLetter();

LetterEditPage letterEditPage = new LetterEditPage(driver);
}
Comment on lines +45 to +56
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

Add assertions to validate test outcomes.

The test method navigates to pages but doesn't verify any expected outcomes. Consider adding assertions to validate that the page was loaded correctly or that specific elements are present.

 @Test
 public void testRandomLetterEditPage() {
     log.info("testRandomLetterEditPage");
     
     MainContentPage mainContentPage = new MainContentPage(driver);
     mainContentPage.pressLetterListLink();

     LetterListPage letterListPage = new LetterListPage(driver);
     letterListPage.pressRandomLetter();

     LetterEditPage letterEditPage = new LetterEditPage(driver);
+    // Verify that we're on the edit page
+    assertTrue(letterEditPage.isPageLoaded(), "Letter edit page failed to load");
 }
📝 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
@Test
public void testRandomLetterEditPage() {
log.info("testRandomLetterEditPage");
MainContentPage mainContentPage = new MainContentPage(driver);
mainContentPage.pressLetterListLink();
LetterListPage letterListPage = new LetterListPage(driver);
letterListPage.pressRandomLetter();
LetterEditPage letterEditPage = new LetterEditPage(driver);
}
@Test
public void testRandomLetterEditPage() {
log.info("testRandomLetterEditPage");
MainContentPage mainContentPage = new MainContentPage(driver);
mainContentPage.pressLetterListLink();
LetterListPage letterListPage = new LetterListPage(driver);
letterListPage.pressRandomLetter();
LetterEditPage letterEditPage = new LetterEditPage(driver);
// Verify that we're on the edit page
assertTrue(letterEditPage.isPageLoaded(), "Letter edit page failed to load");
}


@Test
public void testLetterCreatePage() {
log.info("testLetterCreatePage");

MainContentPage mainContentPage = new MainContentPage(driver);
mainContentPage.pressLetterListLink();

LetterListPage letterListPage = new LetterListPage(driver);
letterListPage.pressCreateButton();

LetterCreatePage letterCreatePage = new LetterCreatePage(driver);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package selenium.content.letter_sound;

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

public class LetterSoundCreatePage {

private WebDriver driver;

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

driver.findElement(By.id("letterSoundCreatePage"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package selenium.content.letter_sound;

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

public class LetterSoundEditPage {

private WebDriver driver;

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

driver.findElement(By.id("letterSoundEditPage"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package selenium.content.letter_sound;

import java.util.List;

import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;

public class LetterSoundListPage {

private WebDriver driver;

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

driver.findElement(By.id("letterSoundListPage"));
}

public void pressRandomLetterSound() {
List<WebElement> links = driver.findElements(By.className("editLink"));
int randomIndex = (int) (Math.random() * links.size());
WebElement randomLink = links.get(randomIndex);
randomLink.click();
}

public void pressCreateButton() {
WebElement button = driver.findElement(By.id("createButton"));
button.click();
}
}
Loading
Loading