You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2021/12/24 16:14:28 UTC

[GitHub] [rocketmq] xijiu opened a new issue #3674: Improve the test cases of the tools module

xijiu opened a new issue #3674:
URL: https://github.com/apache/rocketmq/issues/3674


   # Present Situation
   In the tools module, the test case is closed. `pom.xml` is set up like this
   
   ```
   <properties>
       <maven.test.skip>true</maven.test.skip>
   </properties>
   ```
   
   And some test cases will fail to run, and some test cases are marked as `@Ignore`. The **TARGET** is fix all test cases, and set `maven.test.skip = false`
   
   
   
   # Problem Description
   
   But how to solve it ? The key is to solve the test cases marked by `@Ingore`. Let's look at a current example in class `ProducerConnectionSubCommandTest`
   
   ```
   private static DefaultMQAdminExt defaultMQAdminExt;
   private static DefaultMQAdminExtImpl defaultMQAdminExtImpl;
   private static MQClientInstance mqClientInstance = MQClientManager.getInstance().getOrCreateMQClientInstance(new ClientConfig());
   private static MQClientAPIImpl mQClientAPIImpl;
   
   @BeforeClass
   public static void init() throws NoSuchFieldException, IllegalAccessException, InterruptedException, RemotingTimeoutException, MQClientException, RemotingSendRequestException, RemotingConnectException, MQBrokerException {
       mQClientAPIImpl = mock(MQClientAPIImpl.class);
       defaultMQAdminExt = new DefaultMQAdminExt();
       defaultMQAdminExtImpl = new DefaultMQAdminExtImpl(defaultMQAdminExt, 1000);
   
       Field field = DefaultMQAdminExtImpl.class.getDeclaredField("mqClientInstance");
       field.setAccessible(true);
       ......
   
       ProducerConnection producerConnection = new ProducerConnection();
       Connection connection = new Connection();
       ......
       
       when(mQClientAPIImpl.getProducerConnectionList(anyString(), anyString(), anyLong())).thenReturn(producerConnection);
   }
   
   @Ignore
   @Test
   public void testExecute() throws SubCommandException {
       ProducerConnectionSubCommand cmd = new ProducerConnectionSubCommand();
       Options options = ServerUtil.buildCommandlineOptions(new Options());
       String[] subargs = new String[] {"-g default-producer-group", "-t unit-test"};
       final CommandLine commandLine =
           ServerUtil.parseCmdLine("mqadmin " + cmd.commandName(), subargs, cmd.buildCommandlineOptions(options), new PosixParser());
       cmd.execute(commandLine, options, null);
   }
   ```
   
   
   Obviously, the original author tried to proxy the method `MQClientAPIImpl.getProducerConnectionList` through **Mockito + reflection** and return the mocked data. But the code in the `ProducerConnectionSubCommand` class is indeed like this
   
   ```
   @Override
   public void execute(CommandLine commandLine, Options options, RPCHook rpcHook) throws SubCommandException {
       DefaultMQAdminExt defaultMQAdminExt = new DefaultMQAdminExt(rpcHook);
   
       defaultMQAdminExt.setInstanceName(Long.toString(System.currentTimeMillis()));
   
       try {
           defaultMQAdminExt.start();
           ......
           
       } catch (Exception e) {
           throw new SubCommandException(this.getClass().getSimpleName() + " command failed", e);
       } finally {
           defaultMQAdminExt.shutdown();
       }
   }
   ```
   
   1. `new DefaultMQAdminExt()` created in the method, in the test case, we cannot get its reference
   2. `defaultMQAdminExt.start()` Its member variables will be reassigned
   
   Without changing the code, it is difficult to solve it through **Mockito + reflection**
   
   # Solution
   
   Since it cannot proxy `DefaultMQAdminExt`, just mock the network response data directly. We can create simple netty server for **nameServer** and **broker** respectively. like this
   
   ```
   protected void channelRead0(ChannelHandlerContext ctx, RemotingCommand msg) throws Exception {
       String remark = "mock data";
       final RemotingCommand response =
               RemotingCommand.createResponseCommand(RemotingSysResponseCode.SUCCESS, remark);
       response.setOpaque(msg.getOpaque());
       response.setBody(getBody());
   
       if (extMap != null && extMap.size() > 0) {
           response.setExtFields(extMap);
       }
       ctx.writeAndFlush(response);
   }
   ```
   
   ```
   private ServerResponseMocker startOneBroker() {
       ProducerConnection producerConnection = new ProducerConnection();
       HashSet<Connection> connectionSet = new HashSet<>();
       Connection connection = mock(Connection.class);
       connectionSet.add(connection);
       producerConnection.setConnectionSet(connectionSet);
   
       // start broker
       return ServerResponseMocker.startServer(BROKER_PORT, producerConnection.encode());
   }
   ```
   
   Then specify the PORT and RESPONSE DATA for each test case. The advantage of this is: we only need mock the netty response data, and don't care about the implementation details inside the method
   
   
   
   [jump to pr](https://github.com/apache/rocketmq/pull/3672)
   
   


-- 
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: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] dongeforever commented on issue #3674: Improve the test cases of the tools module

Posted by GitBox <gi...@apache.org>.
dongeforever commented on issue #3674:
URL: https://github.com/apache/rocketmq/issues/3674#issuecomment-1001130285


   It seems great


-- 
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: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] vongosling closed issue #3674: Improve the test cases of the tools module

Posted by GitBox <gi...@apache.org>.
vongosling closed issue #3674:
URL: https://github.com/apache/rocketmq/issues/3674


   


-- 
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: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] dongeforever edited a comment on issue #3674: Improve the test cases of the tools module

Posted by GitBox <gi...@apache.org>.
dongeforever edited a comment on issue #3674:
URL: https://github.com/apache/rocketmq/issues/3674#issuecomment-1001130285


   It seems great!
   Mocking the server is better than mocking the client, if we want to test the code in client side.


-- 
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: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] vongosling commented on issue #3674: Improve the test cases of the tools module

Posted by GitBox <gi...@apache.org>.
vongosling commented on issue #3674:
URL: https://github.com/apache/rocketmq/issues/3674#issuecomment-1001310863


   Keep a close watch for your work, come on~


-- 
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: dev-unsubscribe@rocketmq.apache.org

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