You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/01 01:18:28 UTC

[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10294: [Fix] fix the HttpUtilsTest' test case , it is always time out.

kezhenxu94 commented on code in PR #10294:
URL: https://github.com/apache/dolphinscheduler/pull/10294#discussion_r886245086


##########
dolphinscheduler-common/src/test/java/org/apache/dolphinscheduler/common/utils/HttpUtilsTest.java:
##########
@@ -40,10 +40,10 @@ public class HttpUtilsTest {
     @Test
     public void testGetTest() {
 	// success
-	String result = HttpUtils.get("https://github.com/manifest.json");
+	String result = HttpUtils.get("https://www.baidu.com/sugrec?prod=pc_his&from=pc_web&json=1&sid=36427_36454_31253_36422_36165_36487_36055_36376_36234_26350_36469_36316&hisdata=&_t=1653904731156&req=2&csor=0");

Review Comment:
   > > Actually I'd just remove this test case or set up an http server before the tests. Unit tests should not rely on external third party services. No matter what links you use there must be someone with poor connections and will timeout.
   > 
   > 
   > 
   > So do you need to merge this pr now, or do I hide these two test cases in this pr because they will cause the test to be unstable
   
   Can you just modify the test to set up an http server with Jetty, and use localhost:port to test this http util? That would be stable and no timeout. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org