Integration tests framework mszczygiel part4 #1804

szczygiel-m · 2023-12-18T15:42:05Z

GroupManagementTest
OfflineRetransmissionManagementTest
ReadOnlyModeTest
SubscriptionManagementTest
TopicManagementTest tests with BrokerOperations

…mework_mszczygiel_part4 # Conflicts: # integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java # integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java

…mework_mszczygiel_part4 # Conflicts: # integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java # integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java # integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java # integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java

moscicky · 2023-12-28T13:02:30Z

...st/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java

@@ -37,7 +37,7 @@ public void shouldListSubscriptionsForOwnerId() {
        Subscription subscription3 = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).withOwner(new OwnerId("Plaintext", "ListSubForOwner - Team B")).build());

        // then
-        assertThat(listSubscriptionsForOwner("ListSubForOwner - Team A")).containsExactly(subscription1.getName(), subscription2.getName());
+        assertThat(listSubscriptionsForOwner("ListSubForOwner - Team A")).contains(subscription1.getName(), subscription2.getName());


Have you changed it because of ordering? If yes - containsOnly is more suitable

moscicky · 2023-12-28T13:06:16Z

...grationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java

+    public static final HermesExtension hermes = new HermesExtension();
+
+    @Test
+    public void shouldEmmitAuditEventWhenGroupCreated() {


nit: emmit should be replaces by emit in test names

moscicky · 2023-12-28T13:17:38Z

.../pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java

+        assertThat(getOfflineRetransmissionTasks().size()).isEqualTo(0);
+
+        // cleanup
+        TestSecurityProvider.setUserIsAdmin(true);


Suggested change

TestSecurityProvider.setUserIsAdmin(true);

TestSecurityProvider.reset();

moscicky · 2023-12-28T13:21:43Z

...ntegrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java

+    }
+
+    @Test
+    public void shouldAllowNonModifyingOperations() {


i think this test should be called shouldAllowModifyingOperations and should actually be performed by not admin user?

moscicky · 2023-12-28T13:22:38Z

...ntegrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java

+        WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName));
+
+        // then
+        response.expectStatus().is5xxServerError();


can we check specifically for SERVICE_UNAVAILABLE?

moscicky · 2023-12-28T13:23:51Z

...ntegrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java

+        WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName));
+
+        // then
+        response.expectStatus().is5xxServerError();


SERVICE_UNAVAILABLE?

moscicky · 2023-12-28T13:24:17Z

...ntegrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java

+import pl.allegro.tech.hermes.management.TestSecurityProvider;
+import pl.allegro.tech.hermes.management.domain.mode.ModeService;
+
+public class ReadOnlyModeTest {


nit: consider using random group names instead of hardcoded ones

moscicky · 2023-12-28T13:36:43Z

...Test/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java

+    }
+
+    @Test
+    public void shouldEmmitAuditEventWhenSubscriptionCreated() {


nit: emit instead of emmit in whole file

moscicky · 2023-12-28T13:56:18Z

...common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java

@@ -98,6 +98,23 @@ public TestSubscriber createSubscriberWithRetry(String message, int delay) {
        return subscriber;
    }

+    public TestSubscriber createSubscriberUnavailable() {


is this needed? I think that createSubscriber(503) will be sufficient

moscicky · 2023-12-28T14:02:35Z

...grationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java

+    }
+
+    @Test
+    public void topicCreationRollbackShouldNotDeleteTopicOnBroker() {


Do you have an idea what is a difference between these two tests? I don't understand where is the Rollback part coming from. Perhaps these two test should be merged?

Previously this test was using multi DC setup, do you think this was not relevant?

moscicky · 2023-12-28T14:28:58Z

...ion-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java

@@ -139,6 +139,8 @@ public void clearManagementData() {
        removeSubscriptions();
        removeTopics();
        removeGroups();
+        waitAtMost(Duration.TEN_SECONDS)


this seems redundant, removeGroups already checks if groups are empty:

private void removeGroups() { GroupService service = management.groupService(); List<Group> groups = service.listGroups(); for (Group group : groups) { service.removeGroup(group.getGroupName(), testUser); } waitAtMost(adjust(Duration.ONE_MINUTE)).until(() -> Assertions.assertThat(service.listGroups().size()).isEqualTo(0) ); }

szczygiel-m added 2 commits December 18, 2023 16:26

OfflineRetransmissionManagementTest and ReadOnlyModeTest

fe84161

szczygiel-m added the enhancement label Dec 18, 2023

szczygiel-m added 5 commits December 27, 2023 13:53

SubscriptionManagementTest

49f4b5e

SubscriptionManagementTest

879b738

GroupManagementTest

15a7dc0

BrokerOperations

a26386d

szczygiel-m marked this pull request as ready for review December 28, 2023 11:36

moscicky self-assigned this Dec 28, 2023

Fixing tests reliability

3dda3c9

szczygiel-m force-pushed the integration_tests_framework_mszczygiel_part4 branch from 4869acb to 3dda3c9 Compare December 28, 2023 12:36

Fixing tests reliability

83b9be9

moscicky requested changes Dec 28, 2023

View reviewed changes

Fixing tests reliability

ab9d51d

moscicky reviewed Dec 28, 2023

View reviewed changes

szczygiel-m added 2 commits December 28, 2023 16:35

CR

12943ce

CR

ece72df

moscicky approved these changes Dec 29, 2023

View reviewed changes

szczygiel-m merged commit 997b7fa into integration_tests_framework Dec 29, 2023
7 checks passed

szczygiel-m deleted the integration_tests_framework_mszczygiel_part4 branch December 29, 2023 11:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration tests framework mszczygiel part4 #1804

Integration tests framework mszczygiel part4 #1804

szczygiel-m commented Dec 18, 2023 •

edited

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023

moscicky Dec 28, 2023 •

edited

moscicky Dec 28, 2023

	TestSecurityProvider.setUserIsAdmin(true);
	TestSecurityProvider.reset();

Integration tests framework mszczygiel part4 #1804

Integration tests framework mszczygiel part4 #1804

Conversation

szczygiel-m commented Dec 18, 2023 • edited

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

moscicky Dec 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szczygiel-m commented Dec 18, 2023 •

edited

moscicky Dec 28, 2023 •

edited