Skip to content
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

Add tests to prescriptions #277

Merged
merged 14 commits into from
Nov 6, 2021
Merged

Add tests to prescriptions #277

merged 14 commits into from
Nov 6, 2021

Conversation

huyuxin0429
Copy link

No description provided.

@huyuxin0429 huyuxin0429 added this to the v1.4 milestone Nov 6, 2021
@huyuxin0429 huyuxin0429 linked an issue Nov 6, 2021 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2021

Codecov Report

Merging #277 (deec35f) into master (3b60080) will increase coverage by 9.10%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #277      +/-   ##
============================================
+ Coverage     54.02%   63.13%   +9.10%     
- Complexity      571      685     +114     
============================================
  Files           118      116       -2     
  Lines          2399     2555     +156     
  Branches        262      301      +39     
============================================
+ Hits           1296     1613     +317     
+ Misses         1021      831     -190     
- Partials         82      111      +29     
Impacted Files Coverage Δ
...eedu/docit/logic/parser/AppointmentBookParser.java 33.33% <0.00%> (ø)
...ava/seedu/docit/model/appointment/Appointment.java 77.04% <ø> (+29.50%) ⬆️
...a/seedu/docit/model/prescription/Prescription.java 76.19% <50.00%> (+32.71%) ⬆️
.../commands/prescription/AddPrescriptionCommand.java 85.10% <72.72%> (+75.10%) ⬆️
...mmands/prescription/DeletePrescriptionCommand.java 83.87% <75.00%> (+83.87%) ⬆️
...ser/prescription/AddPrescriptionCommandParser.java 92.30% <83.33%> (+92.30%) ⬆️
.../prescription/DeletePrescriptionCommandParser.java 92.85% <100.00%> (+92.85%) ⬆️
...in/java/seedu/docit/model/util/SampleDataUtil.java 31.81% <0.00%> (-9.57%) ⬇️
...a/seedu/docit/model/patient/UniquePatientList.java 88.88% <0.00%> (+2.22%) ⬆️
...main/java/seedu/docit/logic/parser/ParserUtil.java 71.83% <0.00%> (+2.81%) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b60080...deec35f. Read the comment docs.

@huyuxin0429 huyuxin0429 closed this Nov 6, 2021
@huyuxin0429 huyuxin0429 reopened this Nov 6, 2021

if (medicineName.isBlank()) {
throw new ParseException("Medicine fields cannot be blank.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For code quality:

  • Can make this a public static variable like: MESSAGE_FAILURE
  • then can re-reference this same message for the DeletePrescriptionCommandParserTest

private final AddPrescriptionCommandParser parser = new AddPrescriptionCommandParser();

@Test
public void parse_allFieldsPresent_success() throws ParseException {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name should start with method name:

  • parseAppointmentCommnad_allFieldsPresent_success()

Same for the rest below

@didymental
Copy link

LGTM after code quality changes made

@huyuxin0429 huyuxin0429 merged commit 750d80b into master Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement for Prescription commands/parsers
3 participants