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

Jump to bottom

Replace testng with junit5 for gateway ha module #62

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Replace testng with junit5 for gateway ha module #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions gateway-ha/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@
</dependency>

<dependency>
<groupId>org.testng</groupId>
<artifactId>testng</artifactId>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
<scope>test</scope>
</dependency>

Expand Down
8 changes: 6 additions & 2 deletions gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.trino.gateway.ha;

import static org.junit.Assert.assertTrue;

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;
import io.trino.gateway.ha.config.DataStoreConfiguration;
Expand All @@ -20,9 +22,11 @@
import okhttp3.RequestBody;
import okhttp3.Response;
import org.javalite.activejdbc.Base;
import org.testng.Assert;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

@Slf4j
@TestInstance(Lifecycle.PER_CLASS)
public class HaGatewayTestUtils {
private static final OkHttpClient httpClient = new OkHttpClient();
private static final Random RANDOM = new Random();
Expand Down Expand Up @@ -114,7 +118,7 @@ public static void setUpBackend(
.post(requestBody)
.build();
Response response = httpClient.newCall(request).execute();
Assert.assertTrue(response.isSuccessful());
assertTrue(response.isSuccessful());
}

@Data
Expand Down
20 changes: 12 additions & 8 deletions gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaMultipleBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
package io.trino.gateway.ha;

import static org.junit.Assert.assertEquals;

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import okhttp3.MediaType;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.RequestBody;
import okhttp3.Response;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

@TestInstance(Lifecycle.PER_CLASS)
public class TestGatewayHaMultipleBackend {
public static final String EXPECTED_RESPONSE1 = "{\"id\":\"testId1\"}";
public static final String EXPECTED_RESPONSE2 = "{\"id\":\"testId2\"}";
Expand All @@ -25,7 +29,7 @@ public class TestGatewayHaMultipleBackend {
private final WireMockServer scheduledBackend =
new WireMockServer(WireMockConfiguration.options().port(backend2Port));

@BeforeClass(alwaysRun = true)
@BeforeAll
public void setup() throws Exception {
HaGatewayTestUtils.prepareMockBackend(adhocBackend, "/v1/statement", EXPECTED_RESPONSE1);
HaGatewayTestUtils.prepareMockBackend(scheduledBackend, "/v1/statement", EXPECTED_RESPONSE2);
Expand Down Expand Up @@ -57,7 +61,7 @@ public void testQueryDeliveryToMultipleRoutingGroups() throws Exception {
.post(requestBody)
.build();
Response response1 = httpClient.newCall(request1).execute();
Assert.assertEquals(response1.body().string(), EXPECTED_RESPONSE1);
assertEquals(EXPECTED_RESPONSE1, response1.body().string());

// When X-Trino-Routing-Group is set in header, query should be routed to cluster under the
// routing group
Expand All @@ -68,10 +72,10 @@ public void testQueryDeliveryToMultipleRoutingGroups() throws Exception {
.addHeader("X-Trino-Routing-Group", "scheduled")
.build();
Response response4 = httpClient.newCall(request4).execute();
Assert.assertEquals(response4.body().string(), EXPECTED_RESPONSE2);
assertEquals(EXPECTED_RESPONSE2, response4.body().string());
}

@AfterClass(alwaysRun = true)
@AfterAll
public void cleanup() {
adhocBackend.stop();
scheduledBackend.stop();
Expand Down
18 changes: 11 additions & 7 deletions gateway-ha/src/test/java/io/trino/gateway/ha/TestGatewayHaSingleBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
package io.trino.gateway.ha;

import static org.junit.Assert.assertEquals;

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import okhttp3.MediaType;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.RequestBody;
import okhttp3.Response;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

@TestInstance(Lifecycle.PER_CLASS)
public class TestGatewayHaSingleBackend {
public static final String EXPECTED_RESPONSE = "{\"id\":\"testId\"}";
int backendPort = 20000 + (int) (Math.random() * 1000);
Expand All @@ -21,7 +25,7 @@ public class TestGatewayHaSingleBackend {
new WireMockServer(WireMockConfiguration.options().port(backendPort));
private final OkHttpClient httpClient = new OkHttpClient();

@BeforeClass(alwaysRun = true)
@BeforeAll
public void setup() throws Exception {
HaGatewayTestUtils.prepareMockBackend(backend, "/v1/statement", EXPECTED_RESPONSE);

Expand All @@ -46,10 +50,10 @@ public void testRequestDelivery() throws Exception {
.post(requestBody)
.build();
Response response = httpClient.newCall(request).execute();
Assert.assertEquals(EXPECTED_RESPONSE, response.body().string());
assertEquals(EXPECTED_RESPONSE, response.body().string());
}

@AfterClass(alwaysRun = true)
@AfterAll
public void cleanup() {
backend.stop();
}
Expand Down
22 changes: 11 additions & 11 deletions gateway-ha/src/test/java/io/trino/gateway/ha/handler/TestQueryIdCachingProxyHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
package io.trino.gateway.ha.handler;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

import java.io.IOException;

import javax.servlet.http.HttpServletRequest;

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.Request;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.mockito.Mockito;
import org.testng.Assert;
import org.testng.annotations.Test;

@TestInstance(Lifecycle.PER_CLASS)
public class TestQueryIdCachingProxyHandler {

@Test
public void testExtractQueryIdFromUrl() throws IOException {
String[] paths = {
Expand All @@ -24,7 +25,7 @@ public void testExtractQueryIdFromUrl() throws IOException {
"/ui/api/query.html?20200416_160256_03078_6b4yt"};
for (String path : paths) {
String queryId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(path, null);
assertEquals(queryId, "20200416_160256_03078_6b4yt");
assertEquals("20200416_160256_03078_6b4yt", queryId);
}
String[] nonPaths = {
"/ui/api/query/myOtherThing",
Expand All @@ -46,8 +47,8 @@ public void testForwardedHostHeaderOnProxyRequest() throws IOException {
Request proxyRequest = httpClient.newRequest("http://localhost:80");
QueryIdCachingProxyHandler.setForwardedHostHeaderOnProxyRequest(mockServletRequest,
proxyRequest);
Assert.assertEquals(proxyRequest.getHeaders().get("Host"), String.format("%s:%s",
backendServer, backendPort));
assertEquals(String.format("%s:%s",
backendServer, backendPort), proxyRequest.getHeaders().get("Host"));
}

@Test
Expand All @@ -58,12 +59,11 @@ public void testUserFromRequest() throws IOException {
String authHeader = "Basic dGVzdDoxMjPCow==";
Mockito.when(req.getHeader(QueryIdCachingProxyHandler.AUTHORIZATION))
.thenReturn(authHeader);
Assert.assertEquals(QueryIdCachingProxyHandler.getQueryUser(req), "test");
assertEquals("test", QueryIdCachingProxyHandler.getQueryUser(req));

String user = "trino_user";
Mockito.when(req.getHeader(QueryIdCachingProxyHandler.USER_HEADER))
.thenReturn(user);
Assert.assertEquals(QueryIdCachingProxyHandler.getQueryUser(req), user);

assertEquals(user, QueryIdCachingProxyHandler.getQueryUser(req));
}
}
56 changes: 34 additions & 22 deletions gateway-ha/src/test/java/io/trino/gateway/ha/router/TestHaGatewayManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
package io.trino.gateway.ha.router;

import static org.junit.jupiter.api.Assertions.assertEquals;

import io.trino.gateway.ha.HaGatewayTestUtils;
import io.trino.gateway.ha.config.DataStoreConfiguration;
import io.trino.gateway.ha.config.ProxyBackendConfiguration;
import io.trino.gateway.ha.persistence.JdbcConnectionManager;
import java.io.File;
import java.util.List;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.jupiter.api.TestMethodOrder;

@Test
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
@TestInstance(Lifecycle.PER_CLASS)
Copy link
Contributor

Choose a reason for hiding this comment

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

These test methods can't be run independently - we should rework them to test from a consistent starting point and use Lifecycle.PER_METHOD or a @BeforeEach method to reset the state of the backend DB.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should this be taken care of in another PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that would be cleaner.

public class TestHaGatewayManager {
private HaGatewayManager haGatewayManager;

@BeforeClass(alwaysRun = true)
@BeforeAll
public void setUp() {
File baseDir = new File(System.getProperty("java.io.tmpdir"));
File tempH2DbDir = new File(baseDir, "h2db-" + System.currentTimeMillis());
Expand All @@ -28,6 +35,8 @@ public void setUp() {
haGatewayManager = new HaGatewayManager(connectionManager);
}

@Test
@Order(1)
public void testAddBackend() {
ProxyBackendConfiguration backend = new ProxyBackendConfiguration();
backend.setActive(true);
Expand All @@ -36,25 +45,27 @@ public void testAddBackend() {
backend.setProxyTo("adhoc1.trino.gateway.io");
backend.setExternalUrl("adhoc1.trino.gateway.io");
ProxyBackendConfiguration updated = haGatewayManager.addBackend(backend);
Assert.assertEquals(updated, backend);
assertEquals(backend, updated);
}

@Test(dependsOnMethods = {"testAddBackend"})
@Test
@Order(2)
public void testGetBackends() {
List<ProxyBackendConfiguration> backends = haGatewayManager.getAllBackends();
Assert.assertEquals(backends.size(), 1);
assertEquals(1, backends.size());

backends = haGatewayManager.getActiveBackends("adhoc");
Assert.assertEquals(backends.size(), 1);
assertEquals(1, backends.size());

backends = haGatewayManager.getActiveBackends("unknown");
Assert.assertEquals(backends.size(), 0);
assertEquals(0, backends.size());

backends = haGatewayManager.getActiveAdhocBackends();
Assert.assertEquals(backends.size(), 1);
assertEquals(1, backends.size());
}

@Test(dependsOnMethods = {"testGetBackends"})
@Test
@Order(3)
public void testUpdateBackend() {
ProxyBackendConfiguration backend = new ProxyBackendConfiguration();
backend.setActive(false);
Expand All @@ -64,7 +75,7 @@ public void testUpdateBackend() {
backend.setExternalUrl("adhoc1.trino.gateway.io");
haGatewayManager.updateBackend(backend);
List<ProxyBackendConfiguration> backends = haGatewayManager.getActiveBackends("adhoc");
Assert.assertEquals(backends.size(), 1);
assertEquals(1, backends.size());

backend.setActive(false);
backend.setRoutingGroup("etl");
Expand All @@ -73,23 +84,24 @@ public void testUpdateBackend() {
backend.setExternalUrl("adhoc2.trino.gateway.io");
haGatewayManager.updateBackend(backend);
backends = haGatewayManager.getActiveBackends("adhoc");
Assert.assertEquals(backends.size(), 0);
assertEquals(0, backends.size());
backends = haGatewayManager.getAllBackends();
Assert.assertEquals(backends.size(), 2);
Assert.assertEquals(backends.get(1).getRoutingGroup(), "etl");
assertEquals(2, backends.size());
assertEquals("etl", backends.get(1).getRoutingGroup());
}

@Test(dependsOnMethods = {"testUpdateBackend"})
@Test
@Order(4)
public void testDeleteBackend() {
List<ProxyBackendConfiguration> backends = haGatewayManager.getAllBackends();
Assert.assertEquals(backends.size(), 2);
Assert.assertEquals(backends.get(1).getRoutingGroup(), "etl");
assertEquals(2, backends.size());
assertEquals("etl", backends.get(1).getRoutingGroup());
haGatewayManager.deleteBackend(backends.get(0).getName());
backends = haGatewayManager.getAllBackends();
Assert.assertEquals(backends.size(), 1);
assertEquals(1, backends.size());
}

@AfterClass(alwaysRun = true)
@AfterAll
public void cleanUp() {
}
}
10 changes: 6 additions & 4 deletions gateway-ha/src/test/java/io/trino/gateway/ha/router/TestHaRoutingManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@
import java.io.File;

import lombok.extern.slf4j.Slf4j;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

@Slf4j
@Test
@TestInstance(Lifecycle.PER_CLASS)
public class TestHaRoutingManager {
RoutingManager haRoutingManager;
GatewayBackendManager backendManager;
QueryHistoryManager historyManager;

@BeforeClass(alwaysRun = true)
@BeforeAll
public void setUp() {
File baseDir = new File(System.getProperty("java.io.tmpdir"));
File tempH2DbDir = new File(baseDir, "h2db-" + System.currentTimeMillis());
Expand Down
Loading