[#2394] feat(IT): Add metalake page to web e2e test #2395

ch3yne · 2024-02-28T13:33:24Z

What changes were proposed in this pull request?

Add web e2e test to homepage.

Why are the changes needed?

Fix: #2394

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

N/A

xunliu · 2024-02-29T02:57:25Z

@ch3yne Please fix Github Action failed.
You can execute ./gradew build in gravitino root directory.

xunliu

@LauraXia123 Please help review this PR, Thanks!

jerryshao · 2024-03-07T08:20:31Z

...on-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/MetalakePageTest.java

+    boolean status = metalakePage.verifyIsCreatedMetalake();
+
+    if (status) {
+      LOG.info("create metalake successful");


We don't have to do the log in java test. Just check the return value is expected or not is enough. @xunliu please help @ch3yne to make this test more java-ish.

xunliu · 2024-03-07T09:31:15Z

...on-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/MetalakePageTest.java

+    boolean status = metalakePage.verifyQueryMetalake();
+
+    if (status) {
+      LOG.info("query metalake successful");
+    } else {
+      Assertions.fail("query metalake failed");
+    }


Please change these code to

Assertions.assertTrue(metalakePage.verifyQueryMetalake());

and other place codes.

xunliu · 2024-03-07T09:34:02Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+
+  public void queryMetalakeAction() {
+    LOG.info("test query metalake action started");
+    enterQueryInput("tes");


what mean is tes?

xunliu · 2024-03-07T09:35:49Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+  }
+
+  public void createManyMetalakesAction() {
+    LOG.info("test create many metalakes action started");


Please remove all LOG.info() and other place codes.

xunliu · 2024-03-07T09:39:27Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+  public void clickAddPropertyBtn() {
+    LOG.info("click add metalake property button");
+    this.addMetalakePropertyBtn.click();
+  }


This function only have one line code, call clickAddPropertyBtn() or this.addMetalakePropertyBtn.click() is same.

and clickNextPageBtn(), clickPrevPageBtn()

xunliu · 2024-03-07T09:42:02Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+    try {
+      List<WebElement> dataList = dataViewer.findElements(By.xpath(".//div[@data-field='name']"));
+
+      if (dataList.size() == 9) {


xunliu · 2024-03-11T07:02:56Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+      String rowPath = "//div[@data-id='" + name + "']";
+      WebElement createdMetalakeRow = driver.findElement(By.xpath(rowPath));
+      boolean isRow = createdMetalakeRow.isDisplayed();
+
+      String linkPath = "//div[@data-field='name']//a[@href='/ui/metalakes?metalake=" + name + "']";
+      WebElement createdMetalakeLink = driver.findElement(By.xpath(linkPath));
+      boolean isLink = createdMetalakeLink.isDisplayed();
+      boolean isText = Objects.equals(createdMetalakeLink.getText(), name);


Please add some comments to describe these codes.

xunliu · 2024-03-11T10:16:20Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+    int retry = 0;
+    int sleepTimeMillis = 1_000;
+
+    while (retry++ < 3) {


The 3 second too short, Please change to 5.

xunliu · 2024-03-11T11:43:21Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+      metalakeDetailsDrawer.isDisplayed();
+      String drawerVisible = metalakeDetailsDrawer.getCssValue("visibility");


It's too sketchy to just check if it's visible or not.
Don't we need check value on each element?

LauraXia123 · 2024-03-11T12:45:46Z

no comments for me

xunliu · 2024-03-12T01:00:30Z

...on-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/MetalakePageTest.java

@@ -74,7 +84,7 @@ public void testCreateMultipleMetalakes() {

    for (int i = 0; i < towPagesCount; i++) {


What mean is towPagesCount?

The default counts of one page is 10, I need create over 11 counts of metalakes to make the Go to next page available.

xunliu · 2024-03-12T01:04:55Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+    } finally {
+      // Back to homepage
+      String xpath = "//button[@data-refer='back-home-btn']";
+      WebElement backHomeBtn = driver.findElement(By.xpath(xpath));


Whether need define backHomeBtn to be a MetalakePage Class vaiable?

I added the backHomeBtn Class in CatalogsPage.java, I will modify when the PR is submited.

xunliu · 2024-03-12T01:06:26Z

...test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/utils/AbstractWebIT.java

@@ -15,7 +15,7 @@
 public class AbstractWebIT extends AbstractIT {
  protected static final Logger LOG = LoggerFactory.getLogger(AbstractWebIT.class);
  protected static WebDriver driver;
-  protected static final long MAX_IMPLICIT_WAIT = 60;
+  protected static final long MAX_IMPLICIT_WAIT = 10;


Why need reduce wait time？

When searching for elements, this implicit wait time is too long, which will increase the time it takes for testing.

xunliu · 2024-03-12T14:35:52Z

...-test/src/test/java/com/datastrato/gravitino/integration/test/web/ui/Pages/MetalakePage.java

+    List<WebElement> errorText =
+        nameField.findElements(By.xpath("//div[contains(@class, 'Mui-error')]"));
+
+    return !errorText.isEmpty();


Please use Strings.isEmpty() function.

This is the element function.

xunliu

LGTM

xunliu · 2024-03-13T02:00:10Z

@ch3yne Please remember fix #2512 when you have time.

ch3yne requested a review from xunliu February 28, 2024 13:33

ch3yne self-assigned this Feb 28, 2024

ch3yne force-pushed the add-e2e-homepage branch from 9f28ef2 to def3500 Compare February 28, 2024 13:39

xunliu changed the title ~~[#2394] feat(web): add web e2e test to homepage~~ [#2394] feat(IT): Add metalake page to web e2e test Feb 29, 2024

xunliu reviewed Feb 29, 2024

View reviewed changes

xunliu requested a review from LauraXia123 February 29, 2024 08:28

ch3yne force-pushed the add-e2e-homepage branch 3 times, most recently from 8aef514 to 72e1f14 Compare March 5, 2024 10:04

ch3yne marked this pull request as ready for review March 5, 2024 10:18

ch3yne force-pushed the add-e2e-homepage branch from 13b0a17 to 0cf2377 Compare March 6, 2024 10:00

jerryshao reviewed Mar 7, 2024

View reviewed changes

xunliu reviewed Mar 7, 2024

View reviewed changes

ch3yne added 14 commits March 11, 2024 11:38

add homepage create metalake test

a757a7a

lint

3b7622d

improve create metalake test

d954f6f

add test view metalake

8236382

add test edit and delete metalake

d61ebd8

add test query metalake

748e87b

add test create many metalakes without verify

1beadd9

fix test create many metalakes

0b92a71

add test create metalake with properties

6d89a60

fix ci test element click

08daf60

add test click link navigate to catalogs page

630d952

improve findelement

9aaef6d

fix ci wait

4284a8c

modified file methods

428d7ae

ch3yne force-pushed the add-e2e-homepage branch from 805c43e to 428d7ae Compare March 11, 2024 03:38

xunliu reviewed Mar 11, 2024

View reviewed changes

modified

887f7f5

xunliu reviewed Mar 11, 2024

View reviewed changes

add comment

bcfa1e9

xunliu reviewed Mar 11, 2024

View reviewed changes

ch3yne added 2 commits March 11, 2024 22:08

add invalid name string params

43c0484

lint

521a757

xunliu reviewed Mar 12, 2024

View reviewed changes

ch3yne added 8 commits March 12, 2024 10:54

add element data-refer property

44a60c4

modified

c50fe0a

modified

3857f8c

modified

f2003ff

modified

647db65

modified

7c9710d

modified

accf8a9

remove test link to catalogs page

5f44171

ch3yne mentioned this pull request Mar 12, 2024

[Bug report] the web e2e test: homepage link to catalogs page errors #2512

Closed

make the error code as a comment

b48fa7e

xunliu reviewed Mar 12, 2024

View reviewed changes

add test invalid metalake name

10fcd9f

xunliu approved these changes Mar 13, 2024

View reviewed changes

xunliu merged commit ddfc4cf into apache:main Mar 13, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#2394] feat(IT): Add metalake page to web e2e test #2395

[#2394] feat(IT): Add metalake page to web e2e test #2395

ch3yne commented Feb 28, 2024

xunliu commented Feb 29, 2024

xunliu left a comment

jerryshao Mar 7, 2024

xunliu Mar 7, 2024

xunliu Mar 7, 2024

xunliu Mar 7, 2024

xunliu Mar 7, 2024

xunliu Mar 7, 2024

xunliu Mar 11, 2024

xunliu Mar 11, 2024

xunliu Mar 11, 2024

LauraXia123 commented Mar 11, 2024

xunliu Mar 12, 2024

ch3yne Mar 12, 2024

xunliu Mar 12, 2024

ch3yne Mar 12, 2024

xunliu Mar 12, 2024

ch3yne Mar 12, 2024

xunliu Mar 12, 2024

ch3yne Mar 12, 2024

xunliu left a comment

xunliu commented Mar 13, 2024

		metalakeDetailsDrawer.isDisplayed();
		String drawerVisible = metalakeDetailsDrawer.getCssValue("visibility");

		@@ -74,7 +84,7 @@ public void testCreateMultipleMetalakes() {

		for (int i = 0; i < towPagesCount; i++) {

[#2394] feat(IT): Add metalake page to web e2e test #2395

[#2394] feat(IT): Add metalake page to web e2e test #2395

Conversation

ch3yne commented Feb 28, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

xunliu commented Feb 29, 2024

xunliu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LauraXia123 commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xunliu left a comment

Choose a reason for hiding this comment

xunliu commented Mar 13, 2024