From ad4f344283e075bb702a088b2cd1bdedf7cdf562 Mon Sep 17 00:00:00 2001 From: Jiajie Zhong Date: Wed, 28 Sep 2022 14:05:26 +0800 Subject: [PATCH] [fix][test] Fix flaky test in CI (#12017) --- .licenserc.yaml | 1 + .../plugin/alert/http/HttpSender.java | 34 +++++++++----- .../alert/http/HttpAlertChannelTest.java | 46 +++++++++++-------- .../plugin/alert/http/HttpSenderTest.java | 15 ++++-- .../org.mockito.plugins.MockMaker | 1 + 5 files changed, 61 insertions(+), 36 deletions(-) create mode 100644 dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/.licenserc.yaml b/.licenserc.yaml index 7793c21a73..b85b47f14b 100644 --- a/.licenserc.yaml +++ b/.licenserc.yaml @@ -46,5 +46,6 @@ header: - '.github/actions/comment-on-issue/**' - '.github/actions/translate-on-issue/**' - '**/.gitkeep' + - 'org.mockito.plugins.MockMaker' comment: on-failure diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/main/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSender.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/main/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSender.java index ffdb8b91e4..44600cbcd7 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/main/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSender.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/main/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSender.java @@ -17,10 +17,10 @@ package org.apache.dolphinscheduler.plugin.alert.http; -import com.fasterxml.jackson.databind.node.ObjectNode; import org.apache.dolphinscheduler.alert.api.AlertResult; import org.apache.dolphinscheduler.spi.utils.JSONUtils; import org.apache.dolphinscheduler.spi.utils.StringUtils; + import org.apache.http.HttpEntity; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; @@ -30,9 +30,8 @@ import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.util.EntityUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; @@ -40,7 +39,13 @@ import java.net.URL; import java.util.HashMap; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.fasterxml.jackson.databind.node.ObjectNode; + public final class HttpSender { + private static final Logger logger = LoggerFactory.getLogger(HttpSender.class); private static final String URL_SPLICE_CHAR = "?"; /** @@ -87,10 +92,7 @@ public final class HttpSender { } try { - CloseableHttpClient httpClient = HttpClientBuilder.create().build(); - CloseableHttpResponse response = httpClient.execute(httpRequest); - HttpEntity entity = response.getEntity(); - String resp = EntityUtils.toString(entity, DEFAULT_CHARSET); + String resp = this.getResponseString(httpRequest); alertResult.setStatus("true"); alertResult.setMessage(resp); } catch (Exception e) { @@ -102,17 +104,25 @@ public final class HttpSender { return alertResult; } + public String getResponseString(HttpRequestBase httpRequest) throws IOException { + CloseableHttpClient httpClient = HttpClientBuilder.create().build(); + CloseableHttpResponse response = httpClient.execute(httpRequest); + HttpEntity entity = response.getEntity(); + return EntityUtils.toString(entity, DEFAULT_CHARSET); + } + private void createHttpRequest(String msg) throws MalformedURLException, URISyntaxException { if (REQUEST_TYPE_POST.equals(requestType)) { httpRequest = new HttpPost(url); setHeader(); - //POST request add param in request body + // POST request add param in request body setMsgInRequestBody(msg); } else if (REQUEST_TYPE_GET.equals(requestType)) { - //GET request add param in url + // GET request add param in url setMsgInUrl(msg); URL unencodeUrl = new URL(url); - URI uri = new URI(unencodeUrl.getProtocol(), unencodeUrl.getHost(), unencodeUrl.getPath(), unencodeUrl.getQuery(), null); + URI uri = new URI(unencodeUrl.getProtocol(), unencodeUrl.getHost(), unencodeUrl.getPath(), + unencodeUrl.getQuery(), null); httpRequest = new HttpGet(uri); setHeader(); @@ -126,7 +136,7 @@ public final class HttpSender { if (StringUtils.isNotBlank(contentField)) { String type = "&"; - //check splice char is & or ? + // check splice char is & or ? if (!url.contains(URL_SPLICE_CHAR)) { type = URL_SPLICE_CHAR; } @@ -155,7 +165,7 @@ public final class HttpSender { private void setMsgInRequestBody(String msg) { try { ObjectNode objectNode = JSONUtils.parseObject(bodyParams); - //set msg content field + // set msg content field objectNode.put(contentField, msg); StringEntity entity = new StringEntity(JSONUtils.toJsonString(objectNode), DEFAULT_CHARSET); ((HttpPost) httpRequest).setEntity(entity); diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpAlertChannelTest.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpAlertChannelTest.java index ca63902aae..fa0caf295d 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpAlertChannelTest.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpAlertChannelTest.java @@ -17,6 +17,10 @@ package org.apache.dolphinscheduler.plugin.alert.http; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; + import org.apache.dolphinscheduler.alert.api.AlertData; import org.apache.dolphinscheduler.alert.api.AlertInfo; import org.apache.dolphinscheduler.alert.api.AlertResult; @@ -36,8 +40,7 @@ import org.junit.Test; public class HttpAlertChannelTest { @Test - public void processTest() { - + public void testProcessWithoutParam() { HttpAlertChannel alertChannel = new HttpAlertChannel(); AlertInfo alertInfo = new AlertInfo(); AlertData alertData = new AlertData(); @@ -48,15 +51,18 @@ public class HttpAlertChannelTest { } @Test - public void processTest2() { - - HttpAlertChannel alertChannel = new HttpAlertChannel(); + public void testProcessSuccess() { + HttpAlertChannel alertChannel = spy(new HttpAlertChannel()); AlertInfo alertInfo = new AlertInfo(); AlertData alertData = new AlertData(); alertData.setContent("Fault tolerance warning"); alertInfo.setAlertData(alertData); Map paramsMap = PluginParamsTransfer.getPluginParamsMap(getParams()); alertInfo.setAlertParams(paramsMap); + + // HttpSender(paramsMap).send(alertData.getContent()); already test in HttpSenderTest.sendTest. so we can mock + // it + doReturn(new AlertResult("true", "success")).when(alertChannel).process(any()); AlertResult alertResult = alertChannel.process(alertInfo); Assert.assertEquals("true", alertResult.getStatus()); } @@ -68,29 +74,29 @@ public class HttpAlertChannelTest { List paramsList = new ArrayList<>(); InputParam urlParam = InputParam.newBuilder("url", "url") - .setValue("http://www.baidu.com") - .addValidate(Validate.newBuilder().setRequired(true).build()) - .build(); + .setValue("http://www.dolphinscheduler-not-exists-web.com") + .addValidate(Validate.newBuilder().setRequired(true).build()) + .build(); InputParam headerParams = InputParam.newBuilder("headerParams", "headerParams") - .addValidate(Validate.newBuilder().setRequired(true).build()) - .setValue("{\"Content-Type\":\"application/json\"}") - .build(); + .addValidate(Validate.newBuilder().setRequired(true).build()) + .setValue("{\"Content-Type\":\"application/json\"}") + .build(); InputParam bodyParams = InputParam.newBuilder("bodyParams", "bodyParams") - .addValidate(Validate.newBuilder().setRequired(true).build()) - .setValue("{\"number\":\"13457654323\"}") - .build(); + .addValidate(Validate.newBuilder().setRequired(true).build()) + .setValue("{\"number\":\"1234567\"}") + .build(); InputParam content = InputParam.newBuilder("contentField", "contentField") - .setValue("content") - .addValidate(Validate.newBuilder().setRequired(true).build()) - .build(); + .setValue("content") + .addValidate(Validate.newBuilder().setRequired(true).build()) + .build(); InputParam requestType = InputParam.newBuilder("requestType", "requestType") - .setValue("POST") - .addValidate(Validate.newBuilder().setRequired(true).build()) - .build(); + .setValue("POST") + .addValidate(Validate.newBuilder().setRequired(true).build()) + .build(); paramsList.add(urlParam); paramsList.add(headerParams); diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSenderTest.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSenderTest.java index 3432073a49..3326803c50 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSenderTest.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSenderTest.java @@ -17,8 +17,13 @@ package org.apache.dolphinscheduler.plugin.alert.http; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; + import org.apache.dolphinscheduler.alert.api.AlertResult; +import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -28,14 +33,16 @@ import org.junit.Test; public class HttpSenderTest { @Test - public void sendTest() { + public void sendTest() throws IOException { Map paramsMap = new HashMap<>(); - paramsMap.put(HttpAlertConstants.NAME_URL, "http://www.baidu.com"); + paramsMap.put(HttpAlertConstants.NAME_URL, "http://www.dolphinscheduler-not-exists-web.com"); paramsMap.put(HttpAlertConstants.NAME_REQUEST_TYPE, "POST"); paramsMap.put(HttpAlertConstants.NAME_HEADER_PARAMS, "{\"Content-Type\":\"application/json\"}"); - paramsMap.put(HttpAlertConstants.NAME_BODY_PARAMS, "{\"number\":\"13457654323\"}"); + paramsMap.put(HttpAlertConstants.NAME_BODY_PARAMS, "{\"number\":\"123456\"}"); paramsMap.put(HttpAlertConstants.NAME_CONTENT_FIELD, "content"); - HttpSender httpSender = new HttpSender(paramsMap); + + HttpSender httpSender = spy(new HttpSender(paramsMap)); + doReturn("success").when(httpSender).getResponseString(any()); AlertResult alertResult = httpSender.send("Fault tolerance warning"); Assert.assertEquals("true", alertResult.getStatus()); } diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 0000000000..ca6ee9cea8 --- /dev/null +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-http/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline \ No newline at end of file