-
Notifications
You must be signed in to change notification settings - Fork 5
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
Improve code coverage #125
Improve code coverage #125
Conversation
Codecov Report
@@ Coverage Diff @@
## master #125 +/- ##
============================================
+ Coverage 72.93% 73.10% +0.17%
- Complexity 587 590 +3
============================================
Files 96 96
Lines 2017 2019 +2
Branches 203 204 +1
============================================
+ Hits 1471 1476 +5
+ Misses 481 479 -2
+ Partials 65 64 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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! Made a small nit about the naming of test methods. Thanks for improving code coverage
src/test/java/seedu/address/logic/parser/KeywordParserTest.java
Outdated
Show resolved
Hide resolved
|
||
// same values -> returns true | ||
GenderPredicate firstPredicateCopy = new GenderPredicate(firstPredicateKeyword); | ||
assertTrue(firstPredicate.equals(firstPredicateCopy)); |
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.
Good job adding comments to make code more readable
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.
Overall looks good, may be you can fix the issues mentioned, before merging into the master.
src/test/java/seedu/address/logic/parser/KeywordParserTest.java
Outdated
Show resolved
Hide resolved
src/main/java/seedu/address/logic/parser/AddressBookParser.java
Outdated
Show resolved
Hide resolved
c68176d
into
AY2324S1-CS2103T-T09-3:master
Add more test cases to improve code coverage as well as a DoctorUtil class for testing AddDoctorCommand and its parser.
Moving forwards, I will try to write more test cases for AddDoctorCommand and its parser.