-
Notifications
You must be signed in to change notification settings - Fork 5
Update DG manual testing and acknowledgement section #219
Update DG manual testing and acknowledgement section #219
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #219 +/- ##
=========================================
Coverage 82.61% 82.61%
Complexity 754 754
=========================================
Files 99 99
Lines 2335 2335
Branches 261 261
=========================================
Hits 1929 1929
Misses 337 337
Partials 69 69 ☔ View full report in Codecov by Sentry. |
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, LGTM! However, I have raised some minor issues that we can discuss 😄
docs/DeveloperGuide.md
Outdated
|
||
1. Double-click the jar file Expected: Shows the GUI with a set of sample contacts. The window size may not be optimum. | ||
2. Double-click the `CampusConnect.jar` file Expected: Shows the GUI with a set of sample contacts. The window size may not be optimum. |
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.
I think you should replace the word optimum
by optimal
in this case because optimum
tends to be used before a noun
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.
sure! fixed in 9c011cc
docs/DeveloperGuide.md
Outdated
1. Owing money to a `Person`: | ||
|
||
1. **Prerequisites:** List all `Persons` in CampusConnect using the `list` command. | ||
|
||
2. **Test case:** `owe 1 10`<br> | ||
**Expected:** The first `Person` in the list owes you $10. Success details shown in the status message. | ||
|
||
3. **Test case:** `owe 1 10.555`<br> | ||
**Expected:** The first `Person` in the list will not owe you $10.555. Error details shown in the status message. | ||
|
||
4. **Test case:** `owe 1 50000`<br> | ||
**Expected:** The first `Person` in the list will not owe you $50000. Error details shown in the status message. |
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.
When we use own
command for a given contact, we ourselves should be the one owing to that person right?
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.
yes, totally agreed! very spot on! fixed in 9c011cc
**Expected:** A new `Person` is successfully created with name "John Doe", phone "98765432", email "johndoe@gmail.com" and address "John street, block 123, #01-01". Success details shown in the status message. Moreover, this new `Person` is visible in CampusConnect. | ||
|
||
2. **Test case:** `add n/John Doe p/98765432 e/johndoe@gmail.com a/`<br> | ||
**Expected:** No new `Person` is created. Error details shown in the status message. |
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.
About your Expected description, while the first sentence is complete, the second one is not full. Is that what you intentionally do?
If not, you can consider change it and the rest
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.
i do personally believe that there is no need to elaborate further on what Person
should have been created here since the expected output is that it will never be created. thus, i omitted the nitty gritty details of the supposed Person
object that should be created to be succinct
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! 🥳
Closes #218