איך לכתוב בדיקות יחידה בצורה נכונה

אני חושב שאף פעם לא יצא שישבתי עם מישהו שהסביר לי איך לכתוב בדיקות נכון.
בתואר אקדמי לא מלמדים את זה ובתעשיה הנושא נלמד בעיקר תוך כדי תנועה.


גם מאמרים באינטרנט וגם אנשים לרוב מדברים על - Arrange-Act-Assert , שיטת שלושת השלבים למבנה של בדיקת יחידה.

אבל מה מעבר לזה? מה חשוב בקוד שכזה (חוץ משהבדיקה תעבוד)?

בעיני לפחות, הדבר החשוב ביותר בבדיקת יחידה הוא שהיא תהיה קצרה, ברורה, נקיה ושיהיה אפשר "להריץ" אותה בעיניים במבט קצר.

לשם כך צריך לענות על שאלה פשוטה - מה העיקר ומה הטפל?
את הקוד שחשוב לראות נחשוף בגאווה ואת כל השאר נחביא על ידי הוצאת פונקציות או על ידי שימוש ב-TestInitialize או ClassInitialize.

במידה ומודול הבדיקות מכיל הרבה פרמוטציות של בדיקה דומה (דבר דיי נפוץ), נשתדל לעשות את המבחנים דומים ככל האפשר כדי לאפשר כניסה חלקה למי שהולך לקרוא את הקוד (אם הם דומים לחלוטין, נשתמש ב-Data Driven Test, מוזמנים לקרוא על כך אך זה לא הנושא של הפוסט).

האלמנטים החשובים במבחן צריכים להיות מול העיניים ורצוי למעלה. חשוב לשמור גם על ריווח מספיק כדי שיהיה קל לקרוא את המבחן.

זכרו תמיד - הקריאות (Readability) היא מעל (כמעט) הכל במקרה של בדיקות. מותר לשכפל קוד ומותר לקצר שמות משתנים כדי להכניס אותם לקונטקסט מצומצם יותר - העיקר שיהיה קריא!

כן כן, שמעתם טוב - מותר לשכפל קוד.

והנה ההסבר: במידה והמבחן נכשל, הדבר החשוב ביותר הוא להבין מהי הבעיה במהירות המירבית. כל ניסיון למנוע שכפול קוד דרך קוד משותף או Design מסובך יוביל את המתכנת לניווט בין מחלקות ומודולים שונים ויוציא אותו לגמרי מהקונטקסט שלשמו הוא נקרא - להבין מהר ובקלות מה הבדיקה עושה ומה לא עובד.
מה גם שכתיבה של קוד מלא תלויות מסובך ומורכב תדרוש תחזוקה ותקשה בעתיד על Refactoring.
בנוסף, תשתית משותפת שתנסה להיות כללית מספיק כדי להתאים למודולים ותסריטים שונים עלולה להכיל לא מעט באגים.

ההמלצה שלי היא לשתף רק מה שהכרחי דרך TestClassBase ולהשתדל לגרום למודולים שונים במערכת להיות בלתי תלויים בקוד הבדיקות שלהם גם במידה ויש חפיפה קלה בין המודולים. 
נא להפעיל שיקול דעת :)

בחזרה לעניין, נסו להבין מה הקוד זה עושה:






















אתם אולי חושבים שזה לא כל כך נורא - אבל מבחינתי זה נורא מאד. לא כל כך נעים לקרוא את קטע הקוד הזה - המון קוד לא כל כך מעניין ולא כל כך קשור לבדיקה עצמה, אתחולים, פקודות ארוכות שגורמות להזזת העיניים מצד לצד ועוד.

ועכשיו תראו איזה קסם.
ככה הקובץ שלי נראה אחרי Refactor קל:




















כל מה שחשוב מול העיניים - שיטת המשלוח, ארץ המקור וארץ היעד.

ישנו ריווח שעוזר על הבנת מבנה הבדיקה - השורה הראשונה מכילה את שיטת המשלוח, לאחר מכן שתי שורות דומות לגבי המקור והיעד, לאחר מכן שתי שורות דומות לגבי השמת ערכים באובייטים, לאחר מכן הפעולה עצמה ואז שלב ה - Assert.
בעיני שורות רווח הן חשובות בדיוק כמו שורות שכתוב בהן משהו.

במבט חטוף אפשר להבין בקלות מה קורה כאן ומה ספיציפית הבדיקה הזו באה לבדוק.

לאיפה כל הקוד המגעיל נעלם? הנה הוא.
סביר להניח שאף אחד לא יפתח את ה-Region ברוב המקרים, כי הוא לא כל כך מעניין בקונטקסט של הבדיקה, אבל הנה מה שמסתתר בפנים - אותו קוד מהבדיקה המקורית:






























השאלות שעלולות להתעורר כאן - האם כדאי להוציא לפונקציה שכל מה שהיא מחזירה היא Instance של משתנה מטיפוס מסוים?
האם כדאי להוציא לפונקציה שכל מה שהיא עושה זה לעטוף פונקציה נוספת, בעלת שם ארוך יותר?

התשובה שלי היא - חד משמעית כן!
ככל שהמבחן מכיל קוד קצר יותר - ככה לרוב הוא קריא יותר. אם אפשר לחסוך קריאה לפונקציה בעלת שם ארוך ששייכת לאובייקט בעל שם ארוך ולהחליף אותה ב-Alias קצר, זה מצוין.

על הדרך תרוויחו את זה שתתאהבו במבחנים שאתם כותבים, תאמינו לי :)

נספח קצר על שם המבחן - אני אישית מעדיף את המבנה הזה:

ModuleName_ActionDescription_ExpectedResult

כאשר החלק השמאלי ביותר מציין את שם המודול שאותו אנחנו בודקים, החלק האמצעי מדבר על מהות הבדיקה עצמה (זהירות עם שימוש בשם של פונקציה, שמות פונקציות עלולות להשתנות...) והצד הימני ביותר מתאר את התוצאה הרצויה (ערך מוחזר, הצלחה, כשלון וכו').

זהו,
עידן

3 תגובות:

  1. אחלה פוסט. מתמצת בצורה טובה איך לכתוב טסטים טובים

    השבמחק
  2. היי עידן, פוסט מצויין. יש לי שתי הערות:

    1 - אתה אומר שמותר לשכפל קוד בטסטים ואני קצת חלוק בנושא. עד לא מזמן הייתי מיושר עם הדעה שלך שבטסטים עדיפה קריאות על DRY, אבל אז גיליתי שאני עובד מאוד קשה כאשר הקוד הנבדק משתנה - במקרה כזה אני צריך לעבור על כל המקומות המושכפלים ולשנות בהתאם כדי שיתאימו לקוד החדש - וזה הרבה פעמים מתיש. אז יש כאן דילמה.

    2 - מה קורה כאשר טסט מסויים רוצה לבדוק תרחיש שבו ה StubTimeService מקבל "EN" ולא "IL"? אני מציע שפשוט אותו טסט ידרוס את ה default configuration בהתאם לצרכיו, כלומר, בדוגמא הזו פשוט תופיע שורה בתחילת הטסט שמייצרת StubTimeService חדש עם "EN" ובעצם דורסת את המקורי.

    השבמחק
    תשובות
    1. היי איציק, תודה על התגובה שלך. הנה דעתי לגבי מה שכתבת:

      1 - כמו כל דבר, צריך למצוא את האיזון ואין אמת מוחלטת. ברוב המקרים שאני נתקלתי בהם, יצירת קוד משותף בטסטים לרוב עושה יותר נזק מאשר תועלת, כיוון שטסט צריך להיות ספיציפי ככל הניתן וברגע שיש סביבה משותפת שמנסה לענות על כמה צרכים, אותה סביבה עם הזמן מכילה לוגיקה כלשהי שדורשת תחזוקה.

      צריך למצוא את האיזון בין מה לשתף ובין מה לא לשתף. אם יש משהו שרלוונטי לכל הטסטים, משהו תשתיתי, הייתי שוקל להכניס אותו ל-TestClassBase. דברים שקשורים יותר ללוגיקה של המוצר - פחות.

      חוץ מזה, בעולם הכלים האוטומטיים כגון Resharper, אפשר לעלות על השכפולים האלה ביתר קלות. בנוסף, בגלל האופי של טסטים שכל טסט הוא יחידה שרצה בנפרד, אפשר להשתלט על המשימה הזו - לפצל אותה בין אנשים, לדחות אותה קצת בעזרת Ignore ועוד ועוד.

      משהו שלקחתי פעם מאיש בדיקות עם המון ידע - שאלתי אותו למה אין עץ אחד שמתאר את כל המערכת והוא ענה לי שכל מודול במערכת צריך עץ קצת שונה עם רמות פירוט שונות וזוית שונה, גם אם יש חפיפה בין הנושאים. אני חושב שתשתית הבדיקות צריכה להתאים את עצמה לזוית שאותו מודול צריך לראות.

      אני עדיין בכת המאמינים של שטסטים צריכים להיות DAMP ככל הניתן(Descriptive And Meaningful Phrases), גם על חשבון DRY. היו לי דיונים בנושא גם עם מתכנתים בנושא וגם עם מנהלים, לרוב אנשים פשוט לא מוכנים לשחוט את הפרה הקדושה של DRY.

      2 - במידה ואותו נתון רלוונטי וחשוב לתרחיש הבדיקות, הוא צריך להיות בתוך הטסט - חשוף וגלוי לכל - והייתי מוותר על Default Configuration כדי שהערך לא יהיה מוסתר באף תרחיש.
      לדעתי - או שתמיד יהיה מוסתר כי פחות רלוונטי, או שתמיד יהיה גלוי. Default Configuration הוא פתרון ביניים שאני פחות אוהב כיוון שהוא יכול ליצור מצב של הנחה שלא כתובה במבחן ועשויה להיות חשובה עבורו.

      אופציה אלגנטית נוספת לפתור את זה היא להוסיף Attribute מעל המבחן וקריאה שלו ב-TestInitialize או ב-TestClassBase. זה כבר עניין של אומנות ;)

      מחק