You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/03/30 09:37:43 UTC

[GitHub] liubao68 closed pull request #624: [SCB-443] fix randomly UT failure of TestProviderQpsFlowControlHandler

liubao68 closed pull request #624: [SCB-443] fix randomly UT failure of TestProviderQpsFlowControlHandler
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/624
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
index 6359a7eec..ea5894887 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestConsumerQpsFlowControlHandler.java
@@ -17,16 +17,23 @@
 
 package org.apache.servicecomb.qps;
 
+import static org.junit.Assert.assertEquals;
+
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 
 import mockit.Deencapsulation;
@@ -43,6 +50,9 @@
 
   OperationMeta operationMeta = Mockito.mock(OperationMeta.class);
 
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
   @Before
   public void setUP() {
     ArchaiusUtils.resetConfig();
@@ -58,6 +68,13 @@ public void afterTest() {
 
   @Test
   public void testQpsController() {
+    // to avoid time influence on QpsController
+    new MockUp<System>() {
+      @Mock
+      long currentTimeMillis() {
+        return 1L;
+      }
+    };
     QpsController qpsController = new QpsController("abc", 100);
     Assert.assertEquals(false, qpsController.isLimitNewRequest());
 
@@ -66,80 +83,61 @@ public void testQpsController() {
   }
 
   @Test
-  public void testHandleWithException() {
-    boolean validAssert;
-    try {
-      validAssert = true;
-
-      handler.handle(invocation, asyncResp);
-    } catch (Exception e) {
-      validAssert = false;
-    }
-    Assert.assertFalse(validAssert);
-  }
-
-  @Test
-  public void testHandle() {
-    boolean validAssert;
-    try {
-      validAssert = true;
-      String key = "svc.schema.opr";
-      QpsController qpsController = new QpsController("key", 12);
-      Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
-      Mockito.when(operationMeta.getMicroserviceQualifiedName()).thenReturn(key);
-      setQpsController(key, qpsController);
-      new MockUp<QpsController>() {
-        @Mock
-        public boolean isLimitNewRequest() {
-          return true;
-        }
-      };
-
-      new MockUp<QpsControllerManager>() {
-        @Mock
-        protected QpsController create(String qualifiedNameKey) {
-          return qpsController;
-        }
-      };
-      handler.handle(invocation, asyncResp);
-    } catch (Exception e) {
-      e.printStackTrace();
-      validAssert = false;
-    }
-    Assert.assertTrue(validAssert);
+  public void testHandle() throws Exception {
+    String key = "svc.schema.opr";
+    QpsController qpsController = new QpsController("key", 12);
+    Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
+    Mockito.when(operationMeta.getMicroserviceQualifiedName()).thenReturn(key);
+    setQpsController(key, qpsController);
+    new MockUp<QpsController>() {
+      @Mock
+      public boolean isLimitNewRequest() {
+        return true;
+      }
+    };
+
+    new MockUp<QpsControllerManager>() {
+      @Mock
+      protected QpsController create(String qualifiedNameKey) {
+        return qpsController;
+      }
+    };
+
+    handler.handle(invocation, asyncResp);
+
+    ArgumentCaptor<InvocationException> captor = ArgumentCaptor.forClass(InvocationException.class);
+    Mockito.verify(asyncResp).consumerFail(captor.capture());
+    InvocationException invocationException = captor.getValue();
+    assertEquals(QpsConst.TOO_MANY_REQUESTS_STATUS, invocationException.getStatus());
+    assertEquals("rejected by qps flowcontrol",
+        ((CommonExceptionData) invocationException.getErrorData()).getMessage());
   }
 
   @Test
-  public void testHandleIsLimitNewRequestAsFalse() {
-    boolean validAssert;
-    try {
-      validAssert = true;
-      String key = "MicroserviceQualifiedName";
-      QpsController qpsController = new QpsController("key", 12);
-      Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
-      Mockito.when(operationMeta.getMicroserviceQualifiedName()).thenReturn(key);
-      setQpsController(key, qpsController);
-
-      new MockUp<QpsController>() {
-        @Mock
-        public boolean isLimitNewRequest() {
-          return false;
-        }
-      };
-
-      new MockUp<QpsControllerManager>() {
-
-        @Mock
-        protected QpsController create(String qualifiedNameKey) {
-          return qpsController;
-        }
-      };
-      handler.handle(invocation, asyncResp);
-    } catch (Exception e) {
-      e.printStackTrace();
-      validAssert = false;
-    }
-    Assert.assertTrue(validAssert);
+  public void testHandleIsLimitNewRequestAsFalse() throws Exception {
+    String key = "MicroserviceQualifiedName";
+    QpsController qpsController = new QpsController("key", 12);
+    Mockito.when(invocation.getOperationMeta()).thenReturn(operationMeta);
+    Mockito.when(operationMeta.getMicroserviceQualifiedName()).thenReturn(key);
+    setQpsController(key, qpsController);
+
+    new MockUp<QpsController>() {
+      @Mock
+      public boolean isLimitNewRequest() {
+        return false;
+      }
+    };
+
+    new MockUp<QpsControllerManager>() {
+
+      @Mock
+      protected QpsController create(String qualifiedNameKey) {
+        return qpsController;
+      }
+    };
+    handler.handle(invocation, asyncResp);
+
+    Mockito.verify(invocation).next(asyncResp);
   }
 
   private void setQpsController(String key, QpsController qpsController) {
diff --git a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
index 1bdf8a215..52cf625f3 100644
--- a/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
+++ b/handlers/handler-flowcontrol-qps/src/test/java/org/apache/servicecomb/qps/TestProviderQpsFlowControlHandler.java
@@ -17,17 +17,22 @@
 
 package org.apache.servicecomb.qps;
 
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.times;
 
 import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
+import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 
 import mockit.Expectations;
@@ -44,6 +49,9 @@
 
   OperationMeta operationMeta = Mockito.mock(OperationMeta.class);
 
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
   @Before
   public void setUP() {
     ArchaiusUtils.resetConfig();
@@ -71,109 +79,106 @@ public void testGlobalQpsControl(final @Injectable Invocation invocation,
         result = new RuntimeException("test error");
       }
     };
+    mockUpSystemTime();
 
     ProviderQpsFlowControlHandler gHandler = new ProviderQpsFlowControlHandler();
     gHandler.handle(invocation, asyncResp);
 
     Utils.updateProperty(Config.PROVIDER_LIMIT_KEY_GLOBAL, 3);
 
-    int count = 1;
-    try {
-      gHandler.handle(invocation, asyncResp);
-      count++;
-      gHandler.handle(invocation, asyncResp);
-      fail("An exception is expected!");
-    } catch (Exception e) {
-      Assert.assertEquals(2, count);
-      Assert.assertEquals("test error", e.getMessage());
-    }
+    expectedException.expect(RuntimeException.class);
+    expectedException.expectMessage("test error");
+
+    gHandler.handle(invocation, asyncResp);
+    gHandler.handle(invocation, asyncResp);
   }
 
+
   @Test
   public void testQpsController() {
+    mockUpSystemTime();
     QpsController qpsController = new QpsController("abc", 100);
-    Assert.assertEquals(false, qpsController.isLimitNewRequest());
+    assertEquals(false, qpsController.isLimitNewRequest());
 
     qpsController.setQpsLimit(1);
-    Assert.assertEquals(true, qpsController.isLimitNewRequest());
+    assertEquals(true, qpsController.isLimitNewRequest());
   }
 
   @Test
-  public void testHandleWithException() {
-    boolean validAssert;
-    try {
-      Mockito.when(invocation.getContext(Const.SRC_MICROSERVICE)).thenReturn(null);
-
-      validAssert = true;
-      handler.handle(invocation, asyncResp);
-      handler.handle(invocation, asyncResp);
-    } catch (Exception e) {
-      e.printStackTrace();
-      validAssert = false;
-    }
-    Assert.assertTrue(validAssert);
+  public void testHandleOnSourceMicroserviceNameIsNull() throws Exception {
+    Mockito.when(invocation.getContext(Const.SRC_MICROSERVICE)).thenReturn(null);
+
+    handler.handle(invocation, asyncResp);
+    handler.handle(invocation, asyncResp);
+
+    Mockito.verify(invocation, times(2)).next(asyncResp);
   }
 
   @Test
-  public void testHandle() {
-    boolean validAssert;
-    try {
-      validAssert = true;
-      Mockito.when(invocation.getContext(Const.SRC_MICROSERVICE)).thenReturn("test");
-      OperationMeta mockOperationMeta = QpsControllerManagerTest.getMockOperationMeta("pojo", "server", "opr");
-      Mockito.when(invocation.getOperationMeta()).thenReturn(mockOperationMeta);
-
-      new MockUp<QpsController>() {
-        @Mock
-        public boolean isLimitNewRequest() {
-          return true;
-        }
-      };
-
-      new MockUp<QpsControllerManager>() {
-
-        @Mock
-        protected QpsController create(String qualifiedNameKey) {
-          return new QpsController(qualifiedNameKey, 12);
-        }
-      };
-      handler.handle(invocation, asyncResp);
-    } catch (Exception e) {
-      e.printStackTrace();
-      validAssert = false;
-    }
-    Assert.assertTrue(validAssert);
+  public void testHandle() throws Exception {
+    Mockito.when(invocation.getContext(Const.SRC_MICROSERVICE)).thenReturn("test");
+    OperationMeta mockOperationMeta = QpsControllerManagerTest.getMockOperationMeta("pojo", "server", "opr");
+    Mockito.when(invocation.getOperationMeta()).thenReturn(mockOperationMeta);
+
+    new MockUp<QpsController>() {
+      @Mock
+      public boolean isLimitNewRequest() {
+        return true;
+      }
+    };
+
+    new MockUp<QpsControllerManager>() {
+
+      @Mock
+      protected QpsController create(String qualifiedNameKey) {
+        return new QpsController(qualifiedNameKey, 12);
+      }
+    };
+
+    handler.handle(invocation, asyncResp);
+
+    ArgumentCaptor<InvocationException> captor = ArgumentCaptor.forClass(InvocationException.class);
+    Mockito.verify(asyncResp).producerFail(captor.capture());
+
+    InvocationException invocationException = captor.getValue();
+    assertEquals(QpsConst.TOO_MANY_REQUESTS_STATUS, invocationException.getStatus());
+    assertEquals("rejected by qps flowcontrol",
+        ((CommonExceptionData) invocationException.getErrorData()).getMessage());
   }
 
   @Test
-  public void testHandleIsLimitNewRequestAsFalse() {
-    boolean validAssert;
-    try {
-      validAssert = true;
-      Mockito.when(invocation.getContext(Const.SRC_MICROSERVICE)).thenReturn("test");
-      OperationMeta mockOperationMeta = QpsControllerManagerTest
-          .getMockOperationMeta("pojo", "server", "opr");
-      Mockito.when(invocation.getOperationMeta()).thenReturn(mockOperationMeta);
-
-      new MockUp<QpsController>() {
-        @Mock
-        public boolean isLimitNewRequest() {
-          return false;
-        }
-      };
-
-      new MockUp<QpsControllerManager>() {
-
-        @Mock
-        protected QpsController create(String qualifiedNameKey) {
-          return new QpsController(qualifiedNameKey, 12);
-        }
-      };
-      handler.handle(invocation, asyncResp);
-    } catch (Exception e) {
-      e.printStackTrace();
-      validAssert = false;
-    }
-    Assert.assertTrue(validAssert);
+  public void testHandleIsLimitNewRequestAsFalse() throws Exception {
+    Mockito.when(invocation.getContext(Const.SRC_MICROSERVICE)).thenReturn("test");
+    OperationMeta mockOperationMeta = QpsControllerManagerTest
+        .getMockOperationMeta("pojo", "server", "opr");
+    Mockito.when(invocation.getOperationMeta()).thenReturn(mockOperationMeta);
+
+    new MockUp<QpsController>() {
+      @Mock
+      public boolean isLimitNewRequest() {
+        return false;
+      }
+    };
+
+    new MockUp<QpsControllerManager>() {
+
+      @Mock
+      protected QpsController create(String qualifiedNameKey) {
+        return new QpsController(qualifiedNameKey, 12);
+      }
+    };
+    handler.handle(invocation, asyncResp);
+
+    Mockito.verify(invocation).next(asyncResp);
+  }
+
+  private void mockUpSystemTime() {
+    // to avoid time influence on QpsController
+    new MockUp<System>() {
+      @Mock
+      long currentTimeMillis() {
+        return 1L;
+      }
+    };
   }
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services