-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
test(apollo-core): PropertiesUtilTest #4113
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
Codecov Report
@@ Coverage Diff @@
## master #4113 +/- ##
============================================
+ Coverage 52.49% 52.55% +0.05%
- Complexity 2614 2616 +2
============================================
Files 484 484
Lines 15206 15206
Branches 1572 1572
============================================
+ Hits 7983 7992 +9
+ Misses 6671 6660 -11
- Partials 552 554 +2
Continue to review full report at Codecov.
|
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.
Thank you for your time to participate in open source contributions.
Welcome!
apollo-core/src/test/java/com/ctrip/framework/apollo/core/utils/PropertiesUtilTest.java
Outdated
Show resolved
Hide resolved
|
||
@Test | ||
public void TestProperties() throws IOException { | ||
assertTrue("".equals(PropertiesUtil.toString(new Properties()))); |
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.
Use assertEqual or assertEquals instead of assertTrue
|
||
Properties properties = new Properties(); | ||
properties.put("a","aaa"); | ||
assertTrue("a=aaa\r\n".equals(PropertiesUtil.toString(properties)) |
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.
User System.lineSeperator instead of hard code \r\n
\n
|
||
@Test | ||
public void TestFilterComment(){ | ||
StringBuffer sb=new StringBuffer("#aaaaa\nbbb"); |
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.
String.join(System.lineSepeartor, "#aaaaa“, "bbb"); ?
} | ||
|
||
@Test | ||
public void TestFilterComment(){ |
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.
public void TestFilterComment(){ | |
public void testFilterPropertiesComment(){ |
Keep the method name's convention of test case.
@Test | ||
public void TestFilterComment(){ | ||
StringBuffer sb=new StringBuffer("#aaaaa\nbbb"); | ||
PropertiesUtil.filterPropertiesComment(sb); |
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.
You can add more test case to cover all conditional branchs
apollo/apollo-core/src/main/java/com/ctrip/framework/apollo/core/utils/PropertiesUtil.java
Lines 46 to 57 in 9e19898
static boolean filterPropertiesComment(StringBuffer stringBuffer) { | |
//check whether has comment in the first line | |
if (stringBuffer.charAt(0) != '#') { | |
return false; | |
} | |
int commentLineIndex = stringBuffer.indexOf("\n"); | |
if (commentLineIndex == -1) { | |
return false; | |
} | |
stringBuffer.delete(0, commentLineIndex + 1); | |
return true; | |
} |
…s/PropertiesUtilTest.java Co-authored-by: wxq <Anilople@outlook.com>
import java.util.Properties; | ||
|
||
/** | ||
* @author Wu Mingkan(Dalian University of Technology) |
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.
Should delete author in community project.
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.
LGTM
What's the purpose of this PR
I write a test file for \apollo-core\src\main\java\com\ctrip\framework\apollo\core\utils\PropertiesUtils.java
Which issue(s) this PR fixes:
Fixes #3874
Brief changelog
write two test method for the two function in PropertiesUtils.java seperately.
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.