-
Notifications
You must be signed in to change notification settings - Fork 26.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dubbo-2177]Fix the issue: Supplementary unit test #2177 team 3 #2260
Conversation
import org.junit.Assert; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls. avoid import *
import org.apache.dubbo.common.utils.NetUtils; | ||
import org.apache.dubbo.registry.NotifyListener; | ||
import org.apache.log4j.BasicConfigurator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless import
@@ -36,6 +40,46 @@ | |||
private AbstractRegistry abstractRegistry; | |||
private boolean notifySuccess; | |||
|
|||
private static final Map<String,String> parametersProvider=new LinkedHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless map
|
||
private List<URL> getList() { | ||
List<URL> list = new ArrayList<>(); | ||
URL url_1 = new URL("dubbo", "127.0.0.0", 1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use underscore _
, instead, it's fine to name them as url1
, url2
, etc.
@@ -52,8 +96,14 @@ public boolean isAvailable() { | |||
listener = urls -> notifySuccess = true; | |||
//notify flag | |||
notifySuccess = false; | |||
|
|||
LoggerFactory.setLevel(Level.INFO); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't do this, instead, you should modify resources/log4j.xml
* test register | ||
*/ | ||
@Test | ||
public void testRegister() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls. consider to enhance registerTest
, unregisterTest
, subscribeTest
, unsubsribeTest
, etc, instead of introducing new test methods. But your method naming is more recommended, pls. stick to it.
I will solve the conflict and merge part of your change. |
What is the purpose of the change
Fix issue #2177 By BaiJi the 269th team 3
Brief changelog
The following branches have been Unit Tested:
org.apache.dubbo.registry.support.AbstractRegistry#register
org.apache.dubbo.registry.support.AbstractRegistry#unregister
org.apache.dubbo.registry.support.AbstractRegistry#subscribe
org.apache.dubbo.registry.support.AbstractRegistry#unsubscribe
org.apache.dubbo.registry.support.AbstractRegistry#recover
org.apache.dubbo.registry.support.AbstractRegistry#notify
Verifying this change
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX
. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests
&mvn clean test-compile failsafe:integration-test
to make sure unit-test and integration-test pass.