3.5 בדיקות refactoring ותיעוד הרצאה

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

יצירת בדיקות והמגבלות שלהן - generating tests¶

בקשה כללית כמו "תכתוב בדיקות לפונקציה הזו" מייצרת בדרך כלל בדיקות ל-happy path - התרחיש שבו הכל עובד כצפוי. זה שימושי אבל לא מספיק. הערך האמיתי של בדיקות הוא בתפיסת מקרי קצה (edge cases), קלט לא תקין, ותנאי גבול - בדיוק המקומות שבהם מפתחים (ומודלים) נוטים לפספס.

> Write tests for calculateShippingCost. I want you to cover not
  just the normal case but also: zero or negative weight, an address
  abroad vs. domestic, an order that crosses the free-shipping
  threshold exactly, and missing (undefined) input for one of the fields.

def test_calculate_shipping_normal_case():
    assert calculate_shipping_cost(weight=2.5, country="IL") == 25

def test_calculate_shipping_zero_weight():
    assert calculate_shipping_cost(weight=0, country="IL") == 0

def test_calculate_shipping_negative_weight_raises():
    with pytest.raises(ValueError):
        calculate_shipping_cost(weight=-1, country="IL")

def test_calculate_shipping_free_threshold_exact():
    # exactly at the free-shipping threshold - a critical boundary to test
    assert calculate_shipping_cost(weight=5.0, country="IL") == 0

def test_calculate_shipping_missing_country_raises():
    with pytest.raises(TypeError):
        calculate_shipping_cost(weight=2.5, country=None)

הבדיקה "בדיוק בסף" (weight=5.0) חשובה במיוחד - שגיאות off-by-one (> במקום >=) מתחבאות בדיוק בגבולות כאלה, ובדיקה שרק בודקת "משקל גבוה" ו"משקל נמוך" בלי לגעת בגבול המדויק לא תתפוס אותן.

המגבלה: בדיקות שלא בודקות כלום¶

יש דפוס נפוץ ומטעה: בדיקה שעוברת, אבל בעצם לא בודקת התנהגות אמיתית - למשל בדיקה ש-mock-ה את כל התלויות עד שכל מה שנשאר הוא לבדוק שהקוד "קורא לפונקציה שהוא קורא לה". בדיקה כזו תמיד תעבור, גם אם הלוגיקה שבורה לגמרי.

> Go over the tests you wrote and tell me honestly: is there a test
  that would pass even if I deleted the actual logic and returned a
  constant value instead? If so, which one, and why.

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

אחוז כיסוי הוא לא מדד הצלחה¶

כלי רבים מדווחים "אחוז כיסוי" (coverage) - כמה אחוז משורות הקוד רצות במהלך הבדיקות. קל להיסחף ולבקש מהagent "תעלה את הכיסוי ל-90%", אבל זה מדד מטעה בפני עצמו: אפשר להגיע לכיסוי גבוה מאוד עם בדיקות חלשות שרק "עוברות" על הקוד בלי לבדוק שום דבר משמעותי - כמו לקרוא לפונקציה בלי לבדוק את הערך שהיא מחזירה.

> I don't want you to chase coverage percentage. Instead, go over
  calculateShippingCost and identify every logical branch
  (if/else, error handling) that still has no test, and write a
  test that checks a concrete outcome for each branch.

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

test-first עם agent - test-first development¶

בגישת test-first (או TDD - test-driven development), כותבים קודם את הבדיקה שמתארת את ההתנהגות הרצויה, מוודאים שהיא נכשלת (כי הקוד עוד לא קיים), ורק אז כותבים את המימוש עד שהבדיקה עוברת. עם agent, זו גישה חזקה במיוחד כי היא מכריחה הגדרה מדויקת של "מה נחשב נכון" לפני שהמימוש נכתב - וכך מונעת מהagent "להתאים" את ההגדרה של הצלחה למה שהוא בכל מקרה כתב.

> We'll work test-first. First write a test for applyDiscountCode
  that describes: a valid discount code reduces a percentage off the
  price, an expired code is rejected with an error, an already-used
  code is rejected. Don't write the implementation yet.

$ npm test -- discount

 FAIL  src/services/__tests__/discountService.test.ts
  FAIL: applyDiscountCode is not a function

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

> Great, the tests fail as expected. Now implement applyDiscountCode
  so that all the tests pass, without changing the tests themselves.

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

refactoring בטוח עם בדיקות כרשת ביטחון - safe refactoring¶

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

> Before we start refactoring orderService, run all the
  existing tests and confirm to me that they all pass. If there's
  code without test coverage, tell me before you touch it.

אם מתברר שיש קוד ללא כיסוי, יש בחירה: להוסיף בדיקות לפני ה-refactoring (עדיף), או להיזהר במיוחד באזור הזה כי אין רשת ביטחון. הagent שמדלג ישר ל-refactoring בלי לבדוק כיסוי עלול "לשפר" קוד ולשבור התנהגות ששום בדיקה לא הייתה שם כדי לתפוס.

> Refactor orderService: split the create function, which
  is too long, into smaller functions with clear names.
  Important: the external behavior (what the function returns
  and what errors it throws) must stay identical. After every
  change run the tests and make sure nothing breaks.

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

refactoring הוא לא הזמן לשפר לוגיקה¶

מלכודת נפוצה: תוך כדי refactoring, הagent "משפר" גם התנהגות - למשל מתקן מה שנראה לו כמו באג בדרך. זה מסוכן כי זה מערבב שני סוגי שינוי שצריכים להיסקר בנפרד: שינוי מבנה (בטוח, מגובה בבדיקות) ושינוי התנהגות (מסוכן, דורש בדיקה חדשה שמוכיחה את הכוונה).

> If during the refactoring you notice something that looks like a
  bug in the existing behavior - don't fix it now. Flag it for me
  separately and we'll handle it as a separate feature/fix.

יצירה ועדכון תיעוד - documentation and comments¶

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

> Update the documentation (docstrings) of all public functions
  in orderService.ts. Important: base the description on what the
  code actually does - including edge cases the code handles - not
  on what the function name implies.

def calculate_shipping_cost(weight: float, country: str) -> float:
    """
    Computes shipping cost based on weight and destination country.

    Important notes from the actual implementation:
    - weight 0 returns cost 0 (not treated as an error)
    - negative weight raises ValueError
    - orders in Israel with weight 5.0 kg or above (inclusive) are
      exempt from shipping - this is the exact threshold, not "above 5"
    - country=None raises TypeError, not a default value
    """

ההערה על "5.0 ק"ג ומעלה (כולל)" היא בדיוק סוג הפרט שמונע מהמפתח הבא לשבור את הקוד בטעות כשהוא "מתקן" את התנאי ל-> במקום >= כי זה "נראה יותר הגיוני". תיעוד טוב מתעד את הכוונה וגם את הפרטים המפתיעים בהתנהגות בפועל.

הערות בקוד - inline comments¶

לא כל שורה צריכה הערה. הערה שימושית מסבירה למה, לא מה - כי את ה"מה" רואים ישירות בקוד. שווה להנחות את הagent במפורש:

> Add comments only in places where the code doesn't explain
  itself - for example a non-intuitive decision, a temporary
  workaround, or a dependency on external behavior. Don't add a
  comment that just repeats what the code already says.

תיעוד ברמת פרויקט - README ו-CLAUDE.md¶

עד כה דיברנו על תיעוד ברמת פונקציה - docstrings והערות. אבל יש שכבת תיעוד נוספת, ברמת הפרויקט כולו, שגם היא נהנית מאוד מעדכון שוטף על ידי agent: קובץ README, ובפרויקטים שעובדים עם Claude Code גם קובץ CLAUDE.md שנותן לagent context קבוע על המבנה, הקונבנציות והכללים של הפרויקט (נכיר אותו לעומק בהמשך הקורס).

הבעיה עם README היא שהוא נוטה "להתיישן" - נכתב פעם אחת בתחילת הפרויקט, ואז אף אחד לא מעדכן אותו כשהמבנה משתנה. הagent שקורא את הקוד בפועל יכול לזהות בדיוק איפה README כבר לא תואם למציאות:

> Compare what's written in the README against the actual project
  structure. Tell me exactly which parts are out of date - run
  commands that changed, folders that no longer exist, dependencies
  that were removed.

< Found three inconsistencies:
  1. README mentions "npm start" but package.json defines
     the script under the name "npm run dev"
  2. README describes the src/utils folder, which was already
     split into src/lib and src/helpers
  3. README doesn't mention the REDIS_URL environment variable that
     was added in the last 3 months and is required to run locally

עדכון README על בסיס ממצאים כאלה הוא הרבה יותר אמין מכתיבה מהזיכרון, כי הוא מבוסס על מה שבאמת קיים בקוד עכשיו. שווה להפוך את זה להרגל תקופתי - למשל לפני כל release משמעותי, לבקש מהagent "לבדוק אם התיעוד עדיין תואם את הקוד".

אותו עיקרון חל על CLAUDE.md ומסמכי context דומים: אם קובץ הcontext טוען שהפרויקט משתמש ב-Jest אבל בפועל עברתם ל-Vitest, כל שיחה עתידית עם הagent תתחיל מהנחת יסוד שגויה. תיעוד context שלא מתעדכן לא רק לא עוזר - הוא פעיל מטעה.

> Check if there's anything in CLAUDE.md that's no longer accurate
  about this project - testing tools, folder structure, conventions
  that changed.

דוגמה מלאה: מבדיקה חסרה ל-refactoring בטוח¶

1. > Check the test coverage of discountService - are there
   functions without tests?
   < [identified: applyDiscountCode is not covered at all]

2. > We'll work test-first. Write tests that describe the desired
   behavior, and confirm they currently fail (because there's no
   implementation / the implementation is wrong).
   < [tests fail as expected]

3. > Now refactor: split the function into three clear parts.
   After every change run tests. Don't change behavior.
   < [refactoring done, each test checked after every change]

4. > Update the docstring so it reflects the actual behavior,
   including edge cases.
   < [documentation updated, based on the final code]

סיכום¶

בדיקות שנוצרות על ידי agent נוטות לכסות happy path בלבד אלא אם מבקשים במפורש מקרי קצה, קלט לא תקין וגבולות מדויקים.
מבחן טוב לכל בדיקה: "האם היא הייתה תופסת באג אמיתי, או שהיא תעבור גם עם לוגיקה שבורה?" - שאלו את זה על בדיקות שהagent כתב.
test-first מכריח הגדרה מדויקת של הצלחה לפני מימוש - אבל צריך להנחות במפורש שלא לשנות את הבדיקה כדי שתתאים למימוש הקל.
refactoring דורש חבילת בדיקות ירוקה לפני שמתחילים, הרצת בדיקות אחרי כל שינוי, ואיסור מפורש על ערבוב שינוי מבנה עם שינוי התנהגות.
אם מתגלה קוד ללא כיסוי בדיקות תוך כדי refactoring, זה איתות לעצור ולהוסיף בדיקה לפני שממשיכים, לא לדלג הלאה.
תיעוד טוב מתבסס על מה שהקוד בפועל עושה, כולל edge cases מפתיעים - לא על מה שהשם או הכוונה המקורית מרמזים.
הערות קוד שימושיות מסבירות למה, לא מה - ומיותרות כשהקוד כבר ברור בעצמו.
הכלל המחבר את שלושת הנושאים: בדיקות טובות הן התנאי ל-refactoring בטוח, ותיעוד טוב הוא מה שמונע מהעתיד לשבור בטעות מה שהבדיקות מגנות עליו היום.