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

[#6459]improve(test): Use explicit wait for time-based condition #6460

Merged
merged 2 commits into from
Feb 26, 2025
Merged

[#6459]improve(test): Use explicit wait for time-based condition #6460

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
ff3bc2a
improve(test): Use explicit wait for time-based conditions
zzzk1 Feb 15, 2025
788f3bf
fix format
zzzk1 Feb 25, 2025
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 ...ion-test/src/test/java/org/apache/gravitino/integration/test/web/ui/MetalakePageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void testCreateMultipleMetalakes() throws InterruptedException {

for (int i = 0; i < twoPagesCount; i++) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
Thread.sleep(ACTION_SLEEP * 1000);
} catch (Exception e) {
LOG.error(e.getMessage(), e);
}
Expand Down
257 changes: 118 additions & 139 deletions ...n-test/src/test/java/org/apache/gravitino/integration/test/web/ui/pages/CatalogsPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,8 @@ public boolean verifyShowCatalogDetails(String name, String hiveMetastoreUris)
waitShowText(
"false",
By.xpath(
".//*[@data-prev-refer='details-props-key-gravitino.bypass.hive.metastore.client.capability.check']"));
".//*[@data-prev-refer='details-props-key-gravitino."
+ "bypass.hive.metastore.client.capability.check']"));

boolean verifyAll = isVisible && isText && isHiveURIS && isShowCheck;
if (!verifyAll) {
Expand Down Expand Up @@ -683,79 +684,73 @@ public boolean verifyShowTableTitle(String title) {
*/
public boolean verifyShowPropertiesItemInList(
String item, String key, String value, Boolean isHighlight) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
String xpath;
if (isHighlight) {
xpath = "//div[@data-refer='props-" + item + "-" + key + "-highlight']";
} else {
xpath = "//div[@data-refer='props-" + item + "-" + key + "']";
}
WebElement propertyElement = driver.findElement(By.xpath(xpath));
boolean match = Objects.equals(propertyElement.getText(), value);
WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);
String xpath;
if (isHighlight) {
xpath = "//div[@data-refer='props-" + item + "-" + key + "-highlight']";
} else {
xpath = "//div[@data-refer='props-" + item + "-" + key + "']";
}
WebElement propertyElement =
wait.until(ExpectedConditions.visibilityOfElementLocated(By.xpath(xpath)));

if (!match) {
LOG.error("Prop: does not include itemName: {}", value);
return false;
}
boolean match = Objects.equals(propertyElement.getText(), value);

return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
if (!match) {
LOG.error("Prop: does not include itemName: {}", value);
return false;
}
return true;
}

public boolean verifyShowDataItemInList(String itemName, Boolean isColumnLevel) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
String xpath =
"//div[@data-refer='table-grid']//div[contains(@class, 'MuiDataGrid-main')]/div[contains(@class, 'MuiDataGrid-virtualScroller')]/div/div[@role='rowgroup']//div[@data-field='name']";
if (isColumnLevel) {
xpath = xpath + "//p";
}
List<WebElement> list = driver.findElements(By.xpath(xpath));
List<String> texts = new ArrayList<>();
for (WebElement element : list) {
texts.add(element.getText());
}

if (!texts.contains(itemName)) {
LOG.error("table list: {} does not include itemName: {}", texts, itemName);
return false;
}

return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);
String xpath =
"//div[@data-refer='table-grid']"
+ "//div[contains(@class, 'MuiDataGrid-main')]"
+ "/div[contains(@class, 'MuiDataGrid-virtualScroller')]"
+ "/div/div[@role='rowgroup']//div[@data-field='name']";
if (isColumnLevel) {
xpath = xpath + "//p";
}
List<WebElement> list =
wait.until(ExpectedConditions.visibilityOfAllElementsLocatedBy(By.xpath(xpath)));
List<String> texts = new ArrayList<>();
for (WebElement element : list) {
texts.add(element.getText());
}

if (!texts.contains(itemName)) {
LOG.error("table list: {} does not include itemName: {}", texts, itemName);
return false;
}

return true;
}

public boolean verifyNoDataItemInList(String itemName, Boolean isColumnLevel) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
String xpath =
"//div[@data-refer='table-grid']//div[contains(@class, 'MuiDataGrid-main')]/div[contains(@class, 'MuiDataGrid-virtualScroller')]/div/div[@role='rowgroup']//div[@data-field='name']";
if (isColumnLevel) {
xpath = xpath + "//p";
}
List<WebElement> list = driver.findElements(By.xpath(xpath));
List<String> texts = new ArrayList<>();
for (WebElement element : list) {
texts.add(element.getText());
}

if (texts.contains(itemName)) {
LOG.error("table list: {} does not include itemName: {}", texts, itemName);
return false;
}

return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
String xpath =
"//div[@data-refer='table-grid']"
+ "//div[contains(@class, 'MuiDataGrid-main')]"
+ "/div[contains(@class, 'MuiDataGrid-virtualScroller')]"
+ "/div/div[@role='rowgroup']//div[@data-field='name']";
if (isColumnLevel) {
xpath = xpath + "//p";
}
WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);
List<WebElement> list =
wait.until(ExpectedConditions.visibilityOfAllElementsLocatedBy(By.xpath(xpath)));
List<String> texts = new ArrayList<>();
for (WebElement element : list) {
texts.add(element.getText());
}

if (texts.contains(itemName)) {
LOG.error("table list: {} does not include itemName: {}", texts, itemName);
return false;
}

return true;
}

public boolean verifyTableColumns() {
Expand Down Expand Up @@ -840,106 +835,90 @@ public boolean verifyTablePropertiesOverview(List<String> cols) {
}

public boolean verifyBackHomePage() {
try {
WebDriverWait wait = new WebDriverWait(driver, MAX_TIMEOUT);
wait.until(ExpectedConditions.visibilityOf(metalakePageTitle));
boolean matchTitle = Objects.equals(metalakePageTitle.getText(), "Metalakes");
if (!matchTitle) {
LOG.error(
"metalakePageTitle: {} does not match with Metalakes", metalakePageTitle.getText());
return false;
}
return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
WebDriverWait wait = new WebDriverWait(driver, MAX_TIMEOUT);
wait.until(ExpectedConditions.visibilityOf(metalakePageTitle));
boolean matchTitle = Objects.equals(metalakePageTitle.getText(), "Metalakes");
if (!matchTitle) {
LOG.error("metalakePageTitle: {} does not match with Metalakes", metalakePageTitle.getText());
return false;
}
return true;
}

public boolean verifyRefreshPage() {
try {
WebDriverWait wait = new WebDriverWait(driver, MAX_TIMEOUT);
wait.until(
webDriver ->
((JavascriptExecutor) webDriver)
.executeScript("return document.readyState")
.equals("complete"));
wait.until(ExpectedConditions.visibilityOf(metalakeNameLink));
boolean isDisplayed = metalakeNameLink.isDisplayed();
if (!isDisplayed) {
LOG.error("No match with link, get {}", metalakeNameLink.getText());
return false;
}
return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
WebDriverWait wait = new WebDriverWait(driver, MAX_TIMEOUT);
wait.until(
webDriver ->
((JavascriptExecutor) webDriver)
.executeScript("return document.readyState")
.equals("complete"));
wait.until(ExpectedConditions.visibilityOf(metalakeNameLink));
boolean isDisplayed = metalakeNameLink.isDisplayed();
if (!isDisplayed) {
LOG.error("No match with link, get {}", metalakeNameLink.getText());
return false;
}
return true;
}

public boolean verifyCreatedCatalogs(List<String> catalogNames) {
try {
List<WebElement> list =
tableGrid.findElements(
By.xpath(
"./div[contains(@class, 'MuiDataGrid-main')]/div[contains(@class, 'MuiDataGrid-virtualScroller')]/div/div[@role='rowgroup']//div[@data-field='name']"));
List<String> texts = new ArrayList<>();
for (WebElement webElement : list) {
String rowItemColName = webElement.getText();
texts.add(rowItemColName);
}
if (!texts.containsAll(catalogNames)) {
LOG.error("table list: {} does not containsAll catalogNames: {}", texts, catalogNames);
return false;
}
return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
List<WebElement> list =
tableGrid.findElements(
By.xpath(
"./div[contains(@class, 'MuiDataGrid-main')]"
+ "/div[contains(@class, 'MuiDataGrid-virtualScroller')]"
+ "/div/div[@role='rowgroup']//div[@data-field='name']"));
List<String> texts = new ArrayList<>();
for (WebElement webElement : list) {
String rowItemColName = webElement.getText();
texts.add(rowItemColName);
}
if (!texts.containsAll(catalogNames)) {
LOG.error("table list: {} does not containsAll catalogNames: {}", texts, catalogNames);
return false;
}
return true;
}

public boolean verifyTreeNodes(List<String> treeNodes) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
List<WebElement> list =
driver.findElements(
By.xpath(
"//div[@data-refer='tree-view']//div[@class='ant-tree-list-holder']/div/div[@class='ant-tree-list-holder-inner']/div[contains(@class, 'ant-tree-treenode')]"));
List<String> texts = new ArrayList<>();
for (WebElement webElement : list) {
String nodeName =
webElement.findElement(By.xpath(".//span[@class='ant-tree-title']")).getText();
texts.add(nodeName);
}
if (!treeNodes.containsAll(texts)) {
LOG.error("tree nodes list: {} does not containsAll treeNodes: {}", texts, treeNodes);
return false;
}
return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);
WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);
List<WebElement> list =
wait.until(
ExpectedConditions.visibilityOfAllElementsLocatedBy(
By.xpath(
"//div[@data-refer='tree-view']"
+ "//div[@class='ant-tree-list-holder']"
+ "/div/div[@class='ant-tree-list-holder-inner']"
+ "/div[contains(@class, 'ant-tree-treenode')]")));
List<String> texts = new ArrayList<>();
for (WebElement webElement : list) {
String nodeName =
webElement.findElement(By.xpath(".//span[@class='ant-tree-title']")).getText();
texts.add(nodeName);
}
if (!treeNodes.containsAll(texts)) {
LOG.error("tree nodes list: {} does not containsAll treeNodes: {}", texts, treeNodes);
return false;
}
return true;
}

public boolean verifySelectedNode(String nodeName) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
WebElement selectedNode =
driver.findElement(
By.xpath(
"//div[@data-refer='tree-view']//div[contains(@class, 'ant-tree-treenode-selected')]//span[@class='ant-tree-title']"));
waitShowText(nodeName, selectedNode);
if (!selectedNode.getText().equals(nodeName)) {
LOG.error(
"selectedNode: {} does not match with nodeName: {}", selectedNode.getText(), nodeName);
return false;
}
return true;
} catch (Exception e) {
LOG.error(e.getMessage(), e);

WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);

WebElement selectedNode =
wait.until(
ExpectedConditions.visibilityOfElementLocated(
By.xpath(
"//div[@data-refer='tree-view']"
+ "//div[contains(@class, 'ant-tree-treenode-selected')]"
+ "//span[@class='ant-tree-title']")));
if (!selectedNode.getText().equals(nodeName)) {
LOG.error(
"selectedNode: {} does not match with nodeName: {}", selectedNode.getText(), nodeName);
return false;
}
return true;
}
}
18 changes: 8 additions & 10 deletions ...n-test/src/test/java/org/apache/gravitino/integration/test/web/ui/pages/MetalakePage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,11 @@ public void setMetalakeCommentField(String commentField) {
}

public void setQueryParams(String queryParams) {
try {
Thread.sleep(ACTION_SLEEP_MILLIS);
clearQueryInput();
queryMetalakeInput.sendKeys(queryParams);
Thread.sleep(ACTION_SLEEP_MILLIS);
} catch (Exception e) {
LOG.error(e.getMessage(), e);
}
WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);
WebElement queryInputElement =
wait.until(ExpectedConditions.elementToBeClickable(queryMetalakeInput));
clearQueryInput();
queryInputElement.sendKeys(queryParams);
}

public void clearQueryInput() {
Expand Down Expand Up @@ -187,9 +184,10 @@ public void clickEditMetalakeBtn(String name) {
public void clickMetalakeLink(String name) {
try {
setQueryParams(name);
Thread.sleep(ACTION_SLEEP_MILLIS);
WebDriverWait wait = new WebDriverWait(driver, ACTION_SLEEP);
String xpath = "//a[@data-refer='metalake-link-" + name + "']";
WebElement metalakeLink = metalakeTableGrid.findElement(By.xpath(xpath));
WebElement metalakeLink =
wait.until(ExpectedConditions.elementToBeClickable(By.xpath(xpath)));
clickAndWait(metalakeLink);
} catch (Exception e) {
LOG.error(e.getMessage(), e);
Expand Down
10 changes: 5 additions & 5 deletions ...tion-test/src/test/java/org/apache/gravitino/integration/test/web/ui/utils/BaseWebIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public class BaseWebIT extends BaseIT {
// https://www.selenium.dev/documentation/webdriver/waits/#implicit-waits
protected static final long MAX_IMPLICIT_WAIT = 30;
protected static final long MAX_TIMEOUT = 60;
protected static final long EACH_TEST_SLEEP_MILLIS = 1_000;
protected static final long ACTION_SLEEP_MILLIS = 1_000;
protected static final long EACH_TEST_SLEEP = 1;
protected static final long ACTION_SLEEP = 1;

protected boolean waitShowText(final String text, final Object locator) {
try {
Expand Down Expand Up @@ -82,12 +82,12 @@ protected void clickAndWait(final Object locator) throws InterruptedException {
wait.until(ExpectedConditions.elementToBeClickable(locatorElement(locator)));

locatorElement(locator).click();
Thread.sleep(ACTION_SLEEP_MILLIS);
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue lies in here and L90. You should not delete these two lines unless you have alternatives.

Thread.sleep(ACTION_SLEEP * 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

@zzzk1 @LauraXia123
Can this be replaced with WebDriverWait since we are going to remove Thread.sleep in this PR.

Copy link
Contributor Author

@zzzk1 zzzk1 Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@yuqi1129 I have tried, but there is no way to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zzzk1, Please revert the changes here. I will take a final review.

} catch (ElementClickInterceptedException e) {
// if the previous click did not effected then try clicking in another way
Actions action = new Actions(driver);
action.moveToElement(locatorElement(locator)).click().build().perform();
Thread.sleep(ACTION_SLEEP_MILLIS);
Thread.sleep(ACTION_SLEEP * 1000);
}
}

Expand All @@ -106,7 +106,7 @@ WebElement locatorElement(final Object locatorOrElement) {
@BeforeEach
public void beforeEachTest() {
try {
Thread.sleep(EACH_TEST_SLEEP_MILLIS);
Thread.sleep(EACH_TEST_SLEEP * 1000);
} catch (Exception e) {
LOG.error(e.getMessage(), e);
}
Expand Down