Repository: provectus/kafka-ui
Issue: [e2e] add e2e checks on update/delete topic
Labels: scope/QA


Issue comments:

Pull request review comments:
less words - good
but im not sure, should we be more explicit, what page we assert there

Also, some almost controversial idea - can we try to omit `should`?

It adds opinionated flavour, like lesser brother of `must`. More propositional approach  - `onPage` / `isOnPage` conveys the same amount of information

 I might be wrong.
Specificity helps, it's something around ~`CustomConditions` / at least, `WaitUtils`? Im terrible at names, I only know, that you need to remember contents of utils, as name doesn't help with what utils help

If you want to do it more elegantly, you can try to play with custom conditions, though I hadn't tried it myself 
In case you want, there're probably helpful links
https://github.com/selenide/selenide/wiki/Custom-conditions
https://gitter.im/codeborne/selenide?at=5cb46f4da0790b29c9bdf313
make fields `private` please
You can move  `waitForSelectedValue(topicViewPage.cleanupPolicy, COMPACT_POLICY_VALUE);` into TopicViewPage, so you won't have to expose your internals
Also discussable, as all my other suggestions,
We should probably omit `Page` part of the naming, while enforcing usage only through `pages` entrypoint. Then there won't be any ambiguity, but stream-lined mentally-free usage. Having Page in the name can sometimes  lean into using class by itself, which can result in sloppy design..
if you'll decide to keep it without custom conditions, use class name before using method `SomeUtils.refreshUntil`
so it'll become clearer for next readers. Some can know some selenide basics like `refresh` / `$`, but won't be familiar with custom statics
I guess `refresh` will also work)

that's a misleading method
You actually use `goTo` to go to the direct path(any path/ not topic view),and then just returns topicview for all other `openEditSettings`

Suggest to use
`goTo(path).onTopicListPage()` <--- add assertion which will return needed page

Because you got here ambiguous `openTopicViewPage`
feels like you loading page twice  - on reload and on `goTo` inside `openTopicViewPage`
if you need reload try(with the comment above)
```
pages.reloadPage()
     .onTopicViewPage()
     .openEditSettings()
```

move wait into `topicViewPage` class, don't expose internals
also, try to do it the way it won't require knowledge of the  class fields
small suggestion here. `DELETE_TOPIC` sounds like action,maybe `TOPIC_TO_DELETE` or something like that?
private?
This method is stored in Utils because it is a common method, it will be used not only for topicView.
I'll do it during my next tasks
Done!
Done!
Done!
Done!
Done!
Done!
Done!
Done!
Done!
Done!
Done!
`refreshUntil` until what? In implementation  - until Condition.visible,
suggest to either move condition in arguments ```refreshUntil(By by, Condition condition) {```
or explicitly say `refreshUntilVisible( By by`

Patch that resolved the issue:
diff --git a/kafka-ui-e2e-checks/pom.xml b/kafka-ui-e2e-checks/pom.xml
index 19fb78f2e34..f74ce1a8d62 100644
--- a/kafka-ui-e2e-checks/pom.xml
+++ b/kafka-ui-e2e-checks/pom.xml
@@ -99,7 +99,7 @@
         <dependency>
             <groupId>org.projectlombok</groupId>
             <artifactId>lombok</artifactId>
-            <version>${org.projectlombok.version}</version>
+            <version>${org.projectlombok.e2e-checks.version}</version>
         </dependency>
         <dependency>
             <groupId>org.aspectj</groupId>
diff --git a/pom.xml b/pom.xml
index c91d9e392cb..0c74e664aae 100644
--- a/pom.xml
+++ b/pom.xml
@@ -18,6 +18,7 @@
 		<jackson-databind-nullable.version>0.2.1</jackson-databind-nullable.version>
 		<org.mapstruct.version>1.3.1.Final</org.mapstruct.version>
 		<org.projectlombok.version>1.18.10</org.projectlombok.version>
+		<org.projectlombok.e2e-checks.version>1.18.20</org.projectlombok.e2e-checks.version>
 		<git.revision>latest</git.revision>
 		<zkclient.version>0.11</zkclient.version>
 		<kafka-clients.version>2.4.1</kafka-clients.version>


Test patch:
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/SmokeTests.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/SmokeTests.java
index da7139e434c..1719b0229ca 100644
--- a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/SmokeTests.java
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/SmokeTests.java
@@ -3,7 +3,6 @@
 import com.provectus.kafka.ui.base.BaseTest;
 import io.qameta.allure.Issue;
 import lombok.SneakyThrows;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
@@ -14,7 +13,7 @@ public class SmokeTests extends BaseTest {
     @Issue("380")
     void mainPageLoads() {
         pages.open()
-            .mainPage.shouldBeOnPage();
+                .isOnPage();
         compareScreenshots("main");
     }
 
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/extensions/WaitUtils.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/extensions/WaitUtils.java
new file mode 100644
index 00000000000..f4d18c79775
--- /dev/null
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/extensions/WaitUtils.java
@@ -0,0 +1,31 @@
+package com.provectus.kafka.ui.extensions;
+
+import com.codeborne.selenide.Condition;
+import com.codeborne.selenide.SelenideElement;
+import org.junit.jupiter.api.Assertions;
+import org.openqa.selenium.By;
+
+import static com.codeborne.selenide.Selenide.*;
+import static com.codeborne.selenide.Selenide.$;
+
+public class WaitUtils {
+    public static void refreshUntil(By by, Condition condition) {
+        int i = 0;
+        do {
+            refresh();
+            i++;
+            sleep(2000);
+        } while ($$(by).size() < 1 && i != 20);
+        $(by).shouldBe(condition);
+    }
+
+    public static void waitForSelectedValue(SelenideElement element, String selectedValue) {
+        int i = 0;
+        do {
+            refresh();
+            i++;
+            sleep(2000);
+        } while (!selectedValue.equals(element.getSelectedValue()) && i != 20);
+        Assertions.assertEquals(selectedValue, element.getSelectedValue()) ;
+    }
+}
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/MainPage.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/MainPage.java
index f0749f5fbb5..cb4f922688a 100644
--- a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/MainPage.java
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/MainPage.java
@@ -1,42 +1,38 @@
 package com.provectus.kafka.ui.pages;
 
 import com.codeborne.selenide.Condition;
+import com.codeborne.selenide.Selenide;
+import com.provectus.kafka.ui.base.TestConfiguration;
+import com.provectus.kafka.ui.extensions.WaitUtils;
 import io.qameta.allure.Step;
 import lombok.SneakyThrows;
+import lombok.experimental.ExtensionMethod;
 import org.openqa.selenium.By;
 
 import static com.codeborne.selenide.Selenide.*;
 
+@ExtensionMethod({WaitUtils.class})
 public class MainPage {
 
-  private static final long TIMEOUT = 25000;
+  private static final String path = "";
 
   @Step
-  public MainPage shouldBeOnPage() {
+  public MainPage goTo(){
+    Selenide.open(TestConfiguration.BASE_URL+path);
+    return this;
+  }
+  @Step
+  public MainPage isOnPage() {
     $(By.xpath("//*[contains(text(),'Loading')]")).shouldBe(Condition.disappear);
     $(By.xpath("//h5[text()='Clusters']")).shouldBe(Condition.visible);
     return this;
   }
 
-
-  private void refreshUntil(By by){
-    int i =0;
-    do
-    {
-      refresh();
-      i++;
-      sleep(2000);
-    } while(getElements(by).size()<1 && i!=20);
-    $(by).shouldBe(Condition.visible);
-  }
-
   @SneakyThrows
-  public void shouldBeTopic(String topicName) {
-    refreshUntil(By.xpath("//div[contains(@class,'section')]//table//a[text()='%s']".formatted(topicName)));
+  public void topicIsVisible(String topicName) {
+    By.xpath("//div[contains(@class,'section')]//table//a[text()='%s']".formatted(topicName)).refreshUntil(Condition.visible);
   }
 
-
-
   public enum SideMenuOptions {
     BROKERS("Brokers"),
     TOPICS("Topics"),
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/Pages.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/Pages.java
index 1f21e576707..9b48e097147 100644
--- a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/Pages.java
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/Pages.java
@@ -1,19 +1,27 @@
 package com.provectus.kafka.ui.pages;
 
-import com.codeborne.selenide.Selenide;
-import com.provectus.kafka.ui.base.TestConfiguration;
-
 public class Pages {
 
     public static Pages INSTANCE = new Pages();
 
     public MainPage mainPage = new MainPage();
+    public TopicsList topicsList = new TopicsList();
+    public TopicView topicView = new TopicView();
+
+    public MainPage open() {
+       return openMainPage();
+    }
 
-    private Pages goTo(String path) {
-        Selenide.open(TestConfiguration.BASE_URL+path);
-        return this;
+    public MainPage openMainPage() {
+        return mainPage.goTo();
     }
-    public Pages open() {
-       return goTo("");
+
+    public TopicsList openTopicsList(String clusterName) {
+        return topicsList.goTo(clusterName);
     }
+
+    public TopicView openTopicView(String clusterName, String topicName) {
+        return topicView.goTo(clusterName, topicName);
+    }
+
 }
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/TopicView.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/TopicView.java
new file mode 100644
index 00000000000..386e4626d51
--- /dev/null
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/TopicView.java
@@ -0,0 +1,92 @@
+package com.provectus.kafka.ui.pages;
+
+import com.codeborne.selenide.Selenide;
+import com.codeborne.selenide.SelenideElement;
+import com.provectus.kafka.ui.base.TestConfiguration;
+import com.provectus.kafka.ui.extensions.WaitUtils;
+import io.qameta.allure.Step;
+import lombok.SneakyThrows;
+import lombok.experimental.ExtensionMethod;
+import org.junit.jupiter.api.Assertions;
+import org.openqa.selenium.By;
+
+import static com.codeborne.selenide.Selenide.*;
+
+@ExtensionMethod({WaitUtils.class})
+public class TopicView {
+    private static final String path = "ui/clusters/%s/topics/%s";
+    private final SelenideElement cleanupPolicy = $(By.name("cleanupPolicy"));
+    private final SelenideElement timeToRetain = $(By.id("timeToRetain"));
+    private final SelenideElement maxSizeOnDisk = $(By.name("retentionBytes"));
+    private final SelenideElement maxMessageBytes = $(By.name("maxMessageBytes"));
+
+    @Step
+    public TopicView goTo(String cluster,String topic){
+        Selenide.open(TestConfiguration.BASE_URL+path.formatted(cluster,topic));
+        return this;
+    }
+    @SneakyThrows
+    public TopicView openEditSettings() {
+        $(By.xpath("//a[@class=\"button\" and text()='Edit settings']")).click();
+        return this;
+    }
+
+    @SneakyThrows
+    public void  clickDeleteTopicButton() {
+        $(By.xpath("//*[text()='Delete Topic']")).click();
+        $(By.xpath("//*[text()='Confirm']")).click();
+    }
+
+    @SneakyThrows
+    public TopicView changeCleanupPolicy(String cleanupPolicyValue) {
+        cleanupPolicy.click();
+        $(By.xpath("//select/option[@value = '%s']".formatted(cleanupPolicyValue))).click();
+        return this;
+    }
+
+    @SneakyThrows
+    public TopicView changeTimeToRetainValue(String timeToRetainValue) {
+        timeToRetain.clear();
+        timeToRetain.sendKeys(String.valueOf(timeToRetainValue));
+        return this;
+    }
+
+    @SneakyThrows
+    public TopicView changeMaxSizeOnDisk(String maxSizeOnDiskValue) {
+        maxSizeOnDisk.click();
+        $(By.xpath("//select/option[text() = '%s']".formatted(maxSizeOnDiskValue))).click();
+        return this;
+    }
+
+    @SneakyThrows
+    public TopicView changeMaxMessageBytes(String maxMessageBytesValue) {
+        maxMessageBytes.clear();
+        maxMessageBytes.sendKeys(String.valueOf(maxMessageBytesValue));
+        return this;
+    }
+
+    @SneakyThrows
+    public void submitSettingChanges() {
+        $(By.xpath("//input[@type='submit']")).click();
+    }
+
+    public TopicView cleanupPolicyIs(String value) {
+        cleanupPolicy.waitForSelectedValue(value);
+        return this;
+    }
+
+    public TopicView timeToRetainIs(String time) {
+        Assertions.assertEquals(time, timeToRetain.getValue());
+        return this;
+    }
+
+    public TopicView maxSizeOnDiskIs(String size) {
+        Assertions.assertEquals(size, maxSizeOnDisk.getSelectedText());
+        return this;
+    }
+
+    public TopicView maxMessageBytesIs(String bytes) {
+        Assertions.assertEquals(bytes, maxMessageBytes.getValue());
+        return this;
+    }
+}
\ No newline at end of file
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/TopicsList.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/TopicsList.java
new file mode 100644
index 00000000000..341bfea9fd9
--- /dev/null
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/pages/TopicsList.java
@@ -0,0 +1,47 @@
+package com.provectus.kafka.ui.pages;
+
+import com.codeborne.selenide.Condition;
+import com.codeborne.selenide.Selenide;
+import com.provectus.kafka.ui.base.TestConfiguration;
+import com.provectus.kafka.ui.extensions.WaitUtils;
+import io.qameta.allure.Step;
+import lombok.SneakyThrows;
+import lombok.experimental.ExtensionMethod;
+import org.openqa.selenium.By;
+
+import static com.codeborne.selenide.Selenide.$;
+
+@ExtensionMethod(WaitUtils.class)
+public class TopicsList {
+    private static final String path = "ui/clusters/%s/topics";
+
+    @Step
+    public TopicsList goTo(String cluster) {
+        Selenide.open(TestConfiguration.BASE_URL+path.formatted(cluster));
+        return this;
+    }
+
+    @Step
+    public TopicsList isOnPage() {
+        $(By.xpath("//*[contains(text(),'Loading')]")).shouldBe(Condition.disappear);
+        $(By.xpath("//span[text()='All Topics']")).shouldBe(Condition.visible);
+        return this;
+    }
+
+    @SneakyThrows
+    public TopicsList openTopic(String topicName) {
+        By.xpath("//div[contains(@class,'section')]//table//a[text()='%s']"
+                .formatted(topicName)).refreshUntil(Condition.visible);
+        $(By.xpath("//div[contains(@class,'section')]//table//a[text()='%s']".formatted(topicName)))
+                .click();
+        return this;
+    }
+
+    @SneakyThrows
+    public TopicsList isNotVisible(String topicName) {
+        By.xpath("//div[contains(@class,'section')]//table").refreshUntil(Condition.visible);
+        $(By.xpath("//a[text()='%s']".formatted(topicName))).shouldNotBe(Condition.visible);
+        return this;
+    }
+
+}
diff --git a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/topics/TopicTests.java b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/topics/TopicTests.java
index b32d9b992f1..cfca9d529b7 100644
--- a/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/topics/TopicTests.java
+++ b/kafka-ui-e2e-checks/src/test/java/com/provectus/kafka/ui/topics/TopicTests.java
@@ -1,34 +1,90 @@
 package com.provectus.kafka.ui.topics;
 
+import com.codeborne.selenide.Selenide;
 import com.provectus.kafka.ui.base.BaseTest;
+import com.provectus.kafka.ui.helpers.Helpers;
 import com.provectus.kafka.ui.pages.MainPage;
-import com.provectus.kafka.ui.steps.kafka.KafkaSteps;
-import com.provectus.kafka.ui.helpers.ApiHelper;
 import lombok.SneakyThrows;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.*;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
-public class TopicTests extends BaseTest {
 
+public class TopicTests extends BaseTest {
 
     public static final String NEW_TOPIC = "new-topic";
+    public static final String TOPIC_TO_UPDATE = "topic-to-update";
+    public static final String TOPIC_TO_DELETE = "topic-to-delete";
+    public static final String SECOND_LOCAL = "secondLocal";
+    public static final String COMPACT_POLICY_VALUE = "compact";
+    public static final String UPDATED_TIME_TO_RETAIN_VALUE = "604800001";
+    public static final String UPDATED_MAX_SIZE_ON_DISK = "20 GB";
+    public static final String UPDATED_MAX_MESSAGE_BYTES = "1000020";
+
+    @BeforeAll
+    @SneakyThrows
+    public static void beforeAll() {
+        Helpers.INSTANCE.apiHelper.createTopic(SECOND_LOCAL, TOPIC_TO_UPDATE);
+        Helpers.INSTANCE.apiHelper.createTopic(SECOND_LOCAL, TOPIC_TO_DELETE);
+    }
 
-    @AfterEach
+    @AfterAll
     @SneakyThrows
-        void  afterEach(){
-            helpers.apiHelper.deleteTopic("secondLocal","new-topic");
+    public static void afterAll() {
+        Helpers.INSTANCE.apiHelper.deleteTopic(SECOND_LOCAL, TOPIC_TO_UPDATE);
+        Helpers.INSTANCE.apiHelper.deleteTopic(SECOND_LOCAL, TOPIC_TO_DELETE);
     }
 
     @SneakyThrows
     @DisplayName("should create a topic")
     @Test
-    void createTopic(){
-        helpers.apiHelper.createTopic("secondLocal","new-topic");
-        pages.open()
-                .mainPage.shouldBeOnPage()
-        .goToSideMenu("secondLocal", MainPage.SideMenuOptions.TOPICS)
-        .shouldBeTopic(NEW_TOPIC);
+    void createTopic() {
+        try {
+            helpers.apiHelper.createTopic(SECOND_LOCAL, NEW_TOPIC);
+            pages.open()
+                    .isOnPage()
+                    .goToSideMenu(SECOND_LOCAL, MainPage.SideMenuOptions.TOPICS)
+                    .topicIsVisible(NEW_TOPIC);
+        } finally {
+            helpers.apiHelper.deleteTopic(SECOND_LOCAL, NEW_TOPIC);
+        }
     }
+
+    @SneakyThrows
+    @DisplayName("should update a topic")
+    @Test
+    void updateTopic() {
+        pages.openTopicsList(SECOND_LOCAL)
+                .isOnPage()
+                .openTopic(TOPIC_TO_UPDATE);
+        pages.openTopicView(SECOND_LOCAL, TOPIC_TO_UPDATE)
+                .openEditSettings()
+                .changeCleanupPolicy(COMPACT_POLICY_VALUE)
+                .changeTimeToRetainValue(UPDATED_TIME_TO_RETAIN_VALUE)
+                .changeMaxSizeOnDisk(UPDATED_MAX_SIZE_ON_DISK)
+                .changeMaxMessageBytes(UPDATED_MAX_MESSAGE_BYTES)
+                .submitSettingChanges();
+        Selenide.refresh();
+        pages.openTopicView(SECOND_LOCAL, TOPIC_TO_UPDATE)
+                .openEditSettings()
+        // Assertions
+                .cleanupPolicyIs(COMPACT_POLICY_VALUE)
+                .timeToRetainIs(UPDATED_TIME_TO_RETAIN_VALUE)
+                .maxSizeOnDiskIs(UPDATED_MAX_SIZE_ON_DISK)
+                .maxMessageBytesIs(UPDATED_MAX_MESSAGE_BYTES);
+    }
+
+    @SneakyThrows
+    @DisplayName("should delete topic")
+    @Test
+    @Disabled
+    void deleteTopic() {
+
+        pages.openTopicsList(SECOND_LOCAL)
+                .isOnPage()
+                .openTopic(TOPIC_TO_DELETE);
+        pages.openTopicView(SECOND_LOCAL, TOPIC_TO_DELETE).clickDeleteTopicButton();
+        pages.openTopicsList(SECOND_LOCAL).isNotVisible(TOPIC_TO_DELETE);
+    }
+
 }