Copilot commented on code in PR #4337:
URL: https://github.com/apache/streampark/pull/4337#discussion_r2973648455
##########
streampark-e2e/streampark-e2e-case/src/test/java/org/apache/streampark/e2e/pages/system/SystemPage.java:
##########
@@ -50,37 +51,43 @@ public SystemPage(RemoteWebDriver driver) {
super(driver);
}
+ @SneakyThrows
public <T extends SystemPage.Tab> T goToTab(Class<T> tab) {
if (tab == UserManagementPage.class) {
new WebDriverWait(driver,
Constants.DEFAULT_WEBDRIVER_WAIT_DURATION)
.until(ExpectedConditions.elementToBeClickable(menuUserManagement));
+ Thread.sleep(Constants.DEFAULT_UI_ANIMATION_SLEEP_MILLISECONDS);
menuUserManagement.click();
Review Comment:
`@SneakyThrows` around Thread.sleep will propagate InterruptedException as
an unchecked exception and also clears the interrupt flag. Please catch
InterruptedException explicitly here, call Thread.currentThread().interrupt(),
and then fail fast (e.g., rethrow a RuntimeException) so test
interruptions/shutdowns behave correctly.
##########
streampark-e2e/streampark-e2e-case/src/test/java/org/apache/streampark/e2e/pages/resource/ResourcePage.java:
##########
@@ -44,24 +45,28 @@ public ResourcePage(RemoteWebDriver driver) {
super(driver);
}
+ @SneakyThrows
public <T extends ResourcePage.Tab> T goToTab(Class<T> tab) {
if (tab == VariablesPage.class) {
new WebDriverWait(driver,
Constants.DEFAULT_WEBDRIVER_WAIT_DURATION)
.until(ExpectedConditions.elementToBeClickable(menuVariables));
+ Thread.sleep(Constants.DEFAULT_UI_ANIMATION_SLEEP_MILLISECONDS);
menuVariables.click();
Review Comment:
Using a fixed Thread.sleep before clicking will slow every navigation and
can still be flaky on slower/faster environments. Prefer waiting on a
deterministic condition (e.g., the Resource menu container has the expected
"opened" class / submenu is fully expanded, or retry click until not
intercepted) rather than a hard-coded sleep.
##########
streampark-e2e/streampark-e2e-case/src/test/java/org/apache/streampark/e2e/pages/system/SystemPage.java:
##########
@@ -50,37 +51,43 @@ public SystemPage(RemoteWebDriver driver) {
super(driver);
}
+ @SneakyThrows
public <T extends SystemPage.Tab> T goToTab(Class<T> tab) {
if (tab == UserManagementPage.class) {
new WebDriverWait(driver,
Constants.DEFAULT_WEBDRIVER_WAIT_DURATION)
.until(ExpectedConditions.elementToBeClickable(menuUserManagement));
+ Thread.sleep(Constants.DEFAULT_UI_ANIMATION_SLEEP_MILLISECONDS);
menuUserManagement.click();
Review Comment:
Using a fixed Thread.sleep before clicking will slow every navigation and
can still be flaky on slower/faster environments. Since NavBarPage already
detects expanded menus via the `streampark-menu-opened` class, prefer an
explicit WebDriverWait for the System menu to be in the opened/expanded state
(or retry the click until it succeeds) instead of a hard-coded sleep.
##########
streampark-e2e/streampark-e2e-case/src/test/java/org/apache/streampark/e2e/pages/resource/ResourcePage.java:
##########
@@ -44,24 +45,28 @@ public ResourcePage(RemoteWebDriver driver) {
super(driver);
}
+ @SneakyThrows
public <T extends ResourcePage.Tab> T goToTab(Class<T> tab) {
if (tab == VariablesPage.class) {
new WebDriverWait(driver,
Constants.DEFAULT_WEBDRIVER_WAIT_DURATION)
.until(ExpectedConditions.elementToBeClickable(menuVariables));
+ Thread.sleep(Constants.DEFAULT_UI_ANIMATION_SLEEP_MILLISECONDS);
menuVariables.click();
Review Comment:
`@SneakyThrows` around Thread.sleep will propagate InterruptedException as
an unchecked exception and also clears the interrupt flag. Please catch
InterruptedException explicitly here, call Thread.currentThread().interrupt(),
and then fail fast so test interruptions/shutdowns behave correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
