You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "OrDTesters (GitHub)" <gi...@apache.org> on 2018/11/08 03:56:12 UTC

[GitHub] [incubator-dubbo] OrDTesters opened issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

- [x] I have searched the [issues](https://github.com/apache/incubator-dubbo/issues) of this repository and believe that this is not a duplicate.
- [x] I have checked the [FAQ](https://github.com/apache/incubator-dubbo/blob/master/FAQ.md) of this repository and believe that this is not a duplicate.

### Environment

* Dubbo version: 2.7.x (master)
* Operating System version: Ubuntu 16.04
* Java version: Java HotSpot(TM) 64-Bit Server VM (build 25.181-b13, mixed mode)

### Steps to reproduce this issue

1. `git checkout b0107e767651d066d68c3beaaca9736aed2292b8`
2. `mvn test -am -pl dubbo-rpc/dubbo-rpc-dubbo -Dtest=DubboProtocolTest#testDubboProtocolWithMina -DfailIfNoTests=false`

### Expected Result

The test should pass.

### Actual Result

The test fails. Output:
```
Running org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest
2018-11-08 03:47:05,604 INFO [org.apache.dubbo.common.logger.LoggerFactory:?] - using logger: org.apache.dubbo.common.logger.log4j.Log4jLoggerAdapter
2018-11-08 03:47:06,034 INFO [org.apache.dubbo.rpc.protocol.dubbo.DubboProtocol:destroy] -  [DUBBO] Unexport service: dubbo://127.0.0.1:9010/org.apache.dubbo.rpc.protocol.dubbo.support.DemoService?server=mina, dubbo version: , current host: 172.17.0.12
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.657 sec <<< FAILURE! - in org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest
testDubboProtocolWithMina(org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest)  Time elapsed: 0.086 sec  <<< ERROR!
org.apache.dubbo.rpc.RpcException: Unsupported server type: mina, url: dubbo://127.0.0.1:9010/org.apache.dubbo.rpc.protocol.dubbo.support.DemoService?channel.readonly.sent=true&heartbeat=60000&server=mina
        at org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest.testDubboProtocolWithMina(DubboProtocolTest.java:99)


Results :

Tests in error:
  DubboProtocolTest.testDubboProtocolWithMina:99 ? Rpc Unsupported server type: ...

Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
```

### More Details
1. The test `DubboProtocolTest.testDubboProtocolWithMina` fails when run by itself.
2. The test passes when run in the whole test class, but the test does not actually test anything about mina.
3. A possible fix to make the test pass when run by itself (while still not testing anything about mina) is to apply the following patch:
```
index b67112c..7a30fec 100644
--- a/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocolTest.java
+++ b/dubbo-rpc/dubbo-rpc-dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocolTest.java
@@ -96,6 +96,8 @@ public class DubboProtocolTest {
     @Test
     public void testDubboProtocolWithMina() throws Exception {
         DemoService service = new DemoServiceImpl();
+        protocol.export(proxy.getInvoker(service, DemoService.class, URL.valueOf("dubbo://127.0.0.1:9010/" + DemoService.class.getName())));
+        proxy.getProxy(protocol.refer(DemoService.class, URL.valueOf("dubbo://127.0.0.1:9010/" + DemoService.class.getName()).addParameter("timeout", 3000l)));
         protocol.export(proxy.getInvoker(service, DemoService.class, URL.valueOf("dubbo://127.0.0.1:9010/" + DemoService.class.getName()).addParameter(Constants.SERVER_KEY, "mina")));
         service = proxy.getProxy(protocol.refer(DemoService.class, URL.valueOf("dubbo://127.0.0.1:9010/" + DemoService.class.getName()).addParameter(Constants.CLIENT_KEY, "mina").addParameter("timeout", 3000l)));
         for (int i = 0; i < 10; i++) {
```
4. The test should be changed to actually test something about mina.


[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] carryxyh commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Make this two field locally can solve your problem. It will create a new server

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] carryxyh commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Ah, it is my fault.
We can still use global protocols and proxy.
But we need to modify the URL in the single test case. Take testDubboProtocolWithMina as an example. We need to adjust the port number of the url to ensure that we create a new instance when creating the server and client.
Are you willing to adjust the PR?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] OrDTesters commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "OrDTesters (GitHub)" <gi...@apache.org>.
Thanks for the feedback. From my own testing, it seems only the second step, adding the extra dependency on dubbo-remoting-mina, is enough to get the test to pass when run by itself; we can keep the test class as it is without any changes.

I can send a PR for this issue that only adds the dependency.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] carryxyh commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
U are right.
I have reproduce your problem on my mac. And I have fixed it locally.
Fix method:
1. Make the 'protocol' and 'proxy' to be local variables:

```
public void testDubboProtocolWithMina() throws Exception {
	DemoService service = new DemoServiceImpl();
	Protocol protocol = ExtensionLoader.getExtensionLoader(Protocol.class).getAdaptiveExtension();
	ProxyFactory proxy = ExtensionLoader.getExtensionLoader(ProxyFactory.class).getAdaptiveExtension();
	// ...
}
```

(Same for other test case in DubboProtocolTest)

2. add dependency of dubbo-rpc-dubbo module like this: 

 ```
       <dependency>
            <groupId>org.apache.dubbo</groupId>
            <artifactId>dubbo-remoting-mina</artifactId>
            <version>${project.parent.version}</version>
            <scope>test</scope>
        </dependency>
```

And then it will work well.

What do u think about it? 
If we can reach an agreement, are you willing to submit a pr to fix this problem?


[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] OrDTesters commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "OrDTesters (GitHub)" <gi...@apache.org>.
Thanks for the suggestion. However, I tried making the fields local to the test method as suggested, but from my tracing it seems the two test methods still share the same server (server is only created once). It seems creating a new protocol locally does not make a new DubboProtocol instance, it still grabs the same cached instance from ExtensionLoader.

Aside from the fact that they always share the same server, another question I have is that the two tests, testDubboProtocol and testDubboProtocolWithMina, are almost exactly the same as one another. Should testDubboProtocolWithMina have some different assertions to truly test if using mina is different from regular testDubboProtocol?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] carryxyh closed issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
[ issue closed by carryxyh ]

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] OrDTesters commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "OrDTesters (GitHub)" <gi...@apache.org>.
I have adjusted the PR to change the port number for testDubboProtocolWithMina.

Should the body of testDubboProtocolWithMina be changed to have extra logic that specifically tests for things unique to mina?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] carryxyh commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Make this two protocol locally can solve your problem. It will create a new server

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


[GitHub] [incubator-dubbo] OrDTesters commented on issue #2758: Test DubboProtocolTest.testDubboProtocolWithMina fails when run by itself

Posted by "OrDTesters (GitHub)" <gi...@apache.org>.
I have submitted PR 2797 that adds dubbo-remoting-mina to the module.

However, one concern I have is that before the change, testDubboProtocolWithMina would pass when run after testDubboProtocol in the same test class. When testDubboProtocol runs, it creates a server that is then shared with testDubboProtocolWithMina, so no new server is created when testDubboProtocolWithMina runs afterwards.

The fact that testDubboProtocolWithMina still passes in such a scenario seems weird, suggesting that perhaps the test is not testing anything unique about mina. Should the test be modified more?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/2758 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org