lchokhoe / pe

0 stars 0 forks source link

Patron overdue is a difficult feature to test #7

Open lchokhoe opened 2 years ago

lchokhoe commented 2 years ago

hard-to-test feature because users cannot create a borrow relationship with dates before todays date.

Only way to verify command works is using the sample data.

nus-pe-script commented 2 years ago

Team's Response

We acknowledge that the patron overdue feature is hard to test to a certain extent. However, we do not agree that this should be a medium severity feature flaw, as explained below.

Firstly, there is always tradeoffs when it comes to design choices. An alternative to allow the return date to be earlier than today's date in the borrow command. This does not make any sense in real world application, and can be reported as a larger feature flaw that is not well designed. Our team has chose to prioritize disallowing past dates to prevent misinput, over the less-important testability issue.

Another alternative is to change the borrow command format to be "borrow PATRON_INDEX BOOK_INDEX START_DATE END_DATE". By doing so, the testers can input a start date that is later than end date manually, therefore generating their own overdue books for testing. However, users now need to type an extra "START_DATE". In a practical scenario, a patron will bring the book to the librarian, and the librarian then inputs the borrow command, therefore the borrow date is always today's date. Asking the user to type in one more date will undoubtedly slow the user down a lot, which defeats the purpose of our app (as it is supposed to be a CLI-focused app). Making a CLI-focused app less efficient for fast typists is a much larger feature flaw than testability, and therefore our team made the choice to forgo testability.

With the above points mentioned, our team did make effort to improve testability of our app. For instance, we included this clear warning for users in our UG to tell them not to clear sample data if they intent to test the app

image.png

Also, all our borrowed books in our sample data are intentionally overdue in the first place. There are 3 overdue books, so that even if a book is returned by the tester , there are still 2 other books that are overdue.

Lastly "only way to verify the command is using the sample data" is not exactly accurate, because testers can modify the json file to so that the return date of books is earlier than the borrow date.

Due to the reasons above, we have justified why we prioritize fixing other contradicting feature flaws over testability, and also justified our effort in improving testability.

We initially intend to reject this bug report, but considering that the tester actually puts in effort and tries to test our feature for us, we are accepting this as a low severity bug to appreciate your efforts.

Items for the Tester to Verify

:question: Issue severity

Team chose [severity.Low] Originally [severity.Medium]

Reason for disagreement: Having had the time to think about this after the PE ended, I personally feel that this feature goes past just being hard to test.

I would have to disagree with the first reason that was put forth. From my understanding, this application is meant to serve librarians with their tasking hence the implementation of an "overdue" tag to indicate the existence of overdue books.

Based on the testing, I was not able to (and this is acknowledged by the developers) create an instance of a patron borrowing a book with a due date that is "overdue". While this did make it difficult to test the feature after some thought I would say that this feature flaw is a lot more serious since it hinders the user's ability to accurately input data about the library's patrons and book statuses. Assuming this application was made to be used by an existing NUS library, I think it's reasonable to assume that there are books in the library's catalogues that are in fact currently overdue at the time of keying in data.

The alternative proposed in the second point is also unnecessarily complicated. Simply allowing the user to key in past dates would suffice which means addressing flaws can be done with minimal effort. The developer's point of "preventing miss-input" is also a moot point since a simple "delete" command exists to rectify the mistake (and this to me is reasonable enough of a fix).

I concede that I overlooked testing this feature via making changes directly to the JSON file but my point still stands that users should not need to directly access the JSON file to key such information as an "overdued book".

This is a feature flaw that can impact the utility of the application that has been developed in a significant way. I feel that the severity of Medium is valid since the odds of a library, even an NUS library, having overdue books is too high to overlook (or claimed as not making any sense in the real world). If the overdue feature had been done with active inputs from the user (and not the passive assignment of an "overdue" tag based on the application's implementation), perhaps this would have been a bug that happens on rare occasions. But as it stands, with no other way to work around it, I feel that it's a bug that will hinder users based on the real-world fact that libraries will inadvertently have overdue books.

And, I do not need to "considerations" the development team here. It's unprofessional and I do feel a need to speak out on that. I personally feel that there is a legitimate flaw here and I will justify it.