bugfix: when run server Module ServerTest failed, it occur "hostname can't be null" errors. see issue #743 #907
Conversation
…can't be null" errors. see issue apache#743. According to logics in server.class just add XID set* ops into ServerTest class, then bug is free.
Codecov Report
@@ Coverage Diff @@
## develop #907 +/- ##
==========================================
Coverage 38.33% 38.33%
Complexity 1033 1033
==========================================
Files 222 222
Lines 8664 8664
Branches 1048 1048
==========================================
Hits 3321 3321
Misses 4950 4950
Partials 393 393Continue to review full report at Codecov.
|
| @@ -42,8 +43,9 @@ public static void main(String[] args) { | |||
| RpcServer rpcServer = new RpcServer(workingThreads); | |||
| rpcServer.setHandler(new DefaultCoordinator(rpcServer)); | |||
There was a problem hiding this comment.
please change this class to a test case, not main method.
There was a problem hiding this comment.
Hi, according to your advice, I have done with it. but in server module , the original authors's tests used the Main method, you can view the original code for this , so I want to know that we need to be consistent with him, or should just fix the bug. THX.
There was a problem hiding this comment.
please change this class to a test case, not main method.
for this situation,i thinks it's not necessary to use @Test.otherwise,the server will never be closed.
There was a problem hiding this comment.
you can use @ after to shutdown the test server
There was a problem hiding this comment.
after the server started,the main thread will be block, the @after will not get a chance to execute.
There was a problem hiding this comment.
@xingfudeshi Then should I change the @test method back to Main method which the original author does, and we just focus on the bugfix?
There was a problem hiding this comment.
@xingfudeshi It had already done.
There was a problem hiding this comment.
@leizhiyuan Hi, Add @test and @after method seem not suitable for this case, No as good as we focus on bugfix. So I change back into Main mothod as the original author does.
There was a problem hiding this comment.
@leizhiyuan Hi, since we have common in changes after we discussed before, please approve these changes, not request changes. after this, changes can be merge into develop branch, I think. Thanks all
…can't be null" errors. see issue apache#743. According to logics in server.class just add XID set* ops into ServerTest class, then bug is free. Change: Change ServerTest main method into Test case method.
…can't be null" errors. see issue apache#743. According to logics in server.class just add XID set* ops into ServerTest class, then bug is free. Change: After discussion,when use @test method,It introduces unnecessary content and there is no very effective solution ,SO just focus on bugfux Change Test case method back into Main method.
|
LGTM |
|
Thanks all of you. It's my pleasure. |
…can't be null" errors. see issue apache#743 (apache#907) * bugfix: when run server Module ServerTest failed, it occur "hostname can't be null" errors. see issue apache#743. According to logics in server.class just add XID set* ops into ServerTest class, then bug is free.
Ⅰ. Describe what this PR did
FixBug #743
When run server Module ServerTest failed, it occur "hostname can't be null" errors. see issue seata#743. According to logics in server.class just add XID set* ops into ServerTest class, then bug is free.
Ⅱ. Does this pull request fix one issue?
fixes #743
Ⅲ. Why don't you add test cases (unit test/integration test)?
Because this bug occur in a test class
Ⅳ. Describe how to verify it
just run ServerTest before and after this bugfix.
Ⅴ. Special notes for reviews