DELTA 174 86 2645 SVNŻzÙ}³Z‚€ „„ `ˆ7VŠ$Œ€LCŽ€R=pi’f” ƒU•4`™›€9‚ƒŸ6m˘a*¤[Ĥ€h-¨U€[UŞt€YĴ7`­a€Ħ[} \newcommand{\assessment}[1]{\textbf{Assessment:} #1 \\} \newcommand{\recommendation}[1]{\noindent \textbf{Recommendation:} #1\\} \newcommand{\coverage}[1]{\noindent \textbf{Coverage:} #1} \begin{document} \svnInfo $Id$ \maketitle \section*{Introduction} This document is a summary of the points covered in the review of the Notifier module. It is broken into two sections. The first is the review of the Notifier module design. It is further broken down into subsections in the same format as notifier\_design.txt, namely Private Data Structs, Private Data Members, and Methods. For each component of the design, we include its name, any comments we had on it and a recommendation on how to proceed with the specific component. The second section is the review of the Notifier module test plan. Again we decided to create subsections following the same format as test\_plan.txt: Framework, Acceptance Tests, Black-Box Tests, and White-Box tests. In addition to the descriptors we have used in the first section of this document, we also describe how well the tests covered the particular section they are ascribed to. \section*{Notifier: Design} \subsection*{Introduction} We would first like to present some issues we had with the module as a whole: \begin{itemize} \item The notification periods are global and are not specific for each individual meeting. This was not seen in the requirements. \item The notifier seeems to be unecessarily complex. User preferences should be stored in the User class, and the notifier should be a global class. \item Architecture says that the notifier is only for registered users, and not for unregistered users. \end{itemize \recommendation{Rework} \name{phoneNumber} \comments{See emailAddress} \\ \\ \begin{itemize} \item No indicator of Instant Messing service. \end{itemize} \recommendation{Rework} \name{addNotificationTime} \comments{} \begin{itemize} \item It may be possible to pass around pointers as a solution \end{itemize} \recommendation{Redesign \recommendation{Redesign} \name{rescheduleMeeting} \recommendation{Reimplement} \name{relocateMeeting \recommendation{Rework} \name{cancellationMessage} \comments{See notificationMessage} \\ \\ \recommendation{Rework} \name{relocatMessage} \comments{See notificationMessage} \\Notifier: Test Plan} \subsection*{Framework} \comments{} \begin{itemize} \item Didn't completely understand the xUnit framework \end{itemize} \recommendation{Redesign stubbing} \subsection*{Acceptance Tests} \name{Sends email/IM/SMS} \comments{} \begin{itemize} \item Tests do not check if the messages were sent at the correct time, only if they were sent at all \end{itemize} \recommendation{Redesign} \coverage{} \begin{itemize} \item Load test should be here in the acceptance tests rather than in white-box tests. \end{itemize} \subsection*{Black-Box Tests} \name{Set valid IMNames/email address/phone number/notification preferences} \recommendation{Pass} \name{Adds notification times} \comments{} \begin{itemize} \item Try adding multiple notification times to make sure they are all there \end{itemize} \recommendation{Rework} \name{Removes notification times} \comments{See Adds notification times} \\ \recommendation{Rework} \coverage{} \begin{itemize} \item There are no tests to cover unregistered users \item Should re-examine the specifications to make sure you have good coverage \end{itemize} \subsection*{White-Box Tests} \name{Doesn't set messy IMNames/emails/phone numbers} \comments{} \begin{itemize} \item Needs to check extensively for inputs that are slightly off (i.e. 12 digit number versus 13, one bad character, etc.) \item Using a messy string is a really bad test \end{itemize} \recommendation{Redesign} \name{Constructor fails on bad input} \recommendation{Pass} \name{Note regarding other malformed input} \recommendation{Pass} \name{Constructor sets default value} \recommendation{Pass} \name{Preference can't be irrelevant} \recommendation{Pass} \name{Preferences can't be duplicate} \recommendation{Pass} \name{Preferences can't be empty} \recommendation{Pass} \name{Preferences aren't missed} \recommendation{Pass} \name{Notification times are cloned} \comments{} \begin{itemize} \item In the test, make sure to check the return value is 5 before its 60. \item Couple issues \end{itemize} \recommendation{Redesign} \name{Notification times addition applies retroactively} \comments{} \begin{itemize} \item Does not thoroughly test the two methods (which?), since they're used in different ways. \end{itemize} \recommendation{Re-examine} \name{Notification time deltion applies retroactively} \recommendation{Pass} \name{Module doesn't expose moethod that should be private} \comments{} \begin{itemize} \item It will fail; there are no private methods in the design. \item This test is better performed as a lookover before submitting code for review. \end{itemize} \recommendation{Remove} \name{Moves onto next method if IM fails} \comments{} \begin{itemize} \item Future release, sens message upon complete failure. \end{itemize} \recommendation{Pass} \name{Sends out meeting cancellation notices} \comments{} \begin{itemize} \item Relates to stubbing problem in Framework section. \end{itemize} \recommendation{Redesign} \name{Doesn't notify you of cancelled meetings} \recommendation{Pass} \name{Doesn't send relocation message of past meetings} \recommendation{Pass} \name{Sends out meeting reschheduling notices} \recommendation{Pass} \name{Reschedule meeting I} \recommendation{Rename} \name{Reschedule meeting II} \recommendation{Rename} \name{Sends out simultaneous notifications} \recommendation{Pass} \name{Load Testing} \recommendation{Recategorize to acceptance tests} \coverage{} \begin{itemize} \item Tests are slightly black-box like, but most likely due to design being so high level. \item inform and meeting methods were the trickiest methods and they were not tested. \item Boundary tests exists, but lots of "normal" tests were missing. \item Re-evaluate each method and write more test cases. \end{itemize} \section*{Overall Recommendation} This test plan was not ready for review. The design and test cases took too long to understand how they worked. For the Notifier module, there is a large amount of work necessary to salvage the current design. It is a nontrivial design, and the approach to many methods is conterintuitive, which may be traced back to the architecture. Ultimately, salvaging the current design would not save more time than redesigning the entire module. \end{document} ENDREP id: 3c.0.r176/6780 type: file pred: 3c.0.r175/6536 count: 2 text: 176 0 6755 11412 4b63cae150a7f7bfc82b1c852f653c88 props: 174 2744 29 0 ff5c3c1f7bdb48ba0201950780ae7e31 cpath: /project5/review/review.tex copyroot: 0 / PLAIN K 10 review.tex V 19 file 3c.0.r176/6780 K 23 review_scribe_notes.txt V 20 file 3a.10.r174/2985 END ENDREP id: 3b.0.r176/7113 type: dir pred: 3b.0.r175/6869 count: 2 text: 176 7000 100 100 9f6e1b3555271a31b315ab2089dfe2ec cpath: /project5/review copyroot: 0 / PLAIN K 8 comments V 18 dir 36.0.r173/4541 K 8 packages V 20 dir 2d.0.r167/191632 K 6 review V 18 dir 3b.0.r176/7113 K 12 schedule.txt V 18 file 2c.0.r145/100 END ENDREP id: 2b.0.r176/7437 type: dir pred: 2b.0.r175/7193 count: 33 text: 176 7267 157 157 7c74a6ddd58f1f5c26cf01ac9c422714 cpath: /project5 copyroot: 0 / PLAIN K 8 project2 V 16 dir x.0.r41/3433 K 8 project3 V 17 dir y.0.r113/5763 K 8 project4 V 20 dir 1v.0.r142/151139 K 8 project5 V 18 dir 2b.0.r176/7437 END ENDREP id: 0.0.r176/7749 type: dir pred: 0.0.r175/7505 count: 176 text: 176 7585 151 151 6dba5195a4274d6c9a42d0e1b4c011d1 cpath: / copyroot: 0 / 3c.0.t175-1 modify true false /project5/review/review.tex 7749 7888