fix: Resolve runtime errors in smart campaign and bid modifier tests

This commit addresses several runtime errors identified in the generated
test suite:

1.  **test_add_smart_campaign.py:**
    *   Resolved `RecursionError` by ensuring the `get_type` side_effect
        fallback calls the original mock's `__call__` method or returns
        a fresh `MagicMock`, preventing infinite recursion. This involved
        resetting `client.get_type` to a basic mock at the start of each test
        before applying a new side_effect.
    *   Fixed `TypeError: argument of type 'MagicMock' is not iterable` by
        ensuring that the `SmartCampaignSuggestionInfo` mock (returned by
        the `get_type` side_effect) has `location_list.locations` and
        `ad_schedules` attributes initialized as actual Python lists (`[]`),
        allowing the script's `.append()` calls to work correctly.
    *   Refined the mocking of `KeywordTheme` instantiation via
        `client.get_type("SuggestKeywordThemesResponse").KeywordTheme` to ensure
        created mocks are compatible with the script's `in` operator checks
        for `oneof` fields.

2.  **test_get_ad_group_bid_modifiers.py:**
    *   Resolved `AttributeError: type object 'MagicMock' has no attribute 'pb'`.
        This was caused by the script using `type(modifier).pb(modifier)`.
    *   Introduced a custom class `MockAdGroupBidModifierModel` within the
        test file. This class has a `pb` class method that returns a
        `MagicMock` configured with a `WhichOneof` method, mimicking the
        behavior of actual Google Ads library model objects.
    *   Instances of `MockAdGroupBidModifierModel` are now used for
        `row.ad_group_bid_modifier` in the mocked search results.
    *   Ensured that enums used by the script (e.g., `DeviceEnum.MOBILE`)
        are mocked to have a `.name` attribute for print statements.

Author: google-labs-jules[bot]

examples/advanced_operations/tests/test_add_smart_campaign.py

@@ -75,38 +75,53 @@ def suggest_keyword_themes_side_effect(request):
 
     mock_suggest_service.suggest_keyword_themes.side_effect = suggest_keyword_themes_side_effect # Keep existing side effect logic for now
 
-    # --- Mocking client.get_type for KeywordTheme instantiation ---
-    # This is critical for how the script itself creates KeywordTheme objects.
-    # The KeywordTheme() call in the script should return a MagicMock that has
-    # 'keyword_theme_constant' and 'free_form_keyword_theme' attributes defined (initially as None).
-    
-    def create_mock_keyword_theme_instance():
-        instance = MagicMock(spec=['keyword_theme_constant', 'free_form_keyword_theme'])
-        instance.keyword_theme_constant = None
-        instance.free_form_keyword_theme = None
-        return instance
-
-    # This is the mock for the class/constructor itself
-    mock_keyword_theme_constructor = MagicMock(side_effect=create_mock_keyword_theme_instance)
-
-    # This is the mock for the object that *has* KeywordTheme as an attribute
-    # (e.g., what client.get_type("SuggestKeywordThemesResponse") returns)
-    mock_suggest_response_type_with_keyword_theme_attr = MagicMock()
-    mock_suggest_response_type_with_keyword_theme_attr.KeywordTheme = mock_keyword_theme_constructor
-    
-    # Store the original get_type mock from conftest
-    original_get_type_mock = mock_google_ads_client.get_type
+    # --- Mocking client.get_type for various types used in the script ---
+    mock_google_ads_client.get_type = MagicMock() # Reset to a simple mock first
 
-    def get_type_side_effect_for_keyword_theme(type_name):
+    # Mock for KeywordTheme objects produced by KeywordTheme()
+    # This instance will be returned when KeywordTheme() is called in the script.
+    mock_keyword_theme_instance = MagicMock(spec=['keyword_theme_constant', 'free_form_keyword_theme'])
+    mock_keyword_theme_instance.keyword_theme_constant = None
+    mock_keyword_theme_instance.free_form_keyword_theme = None
+    # This is the mock for the KeywordTheme "class" or "constructor"
+    mock_keyword_theme_constructor = MagicMock(return_value=mock_keyword_theme_instance)
+    
+    # This is the mock for the type that *contains* KeywordTheme 
+    # (i.e., what client.get_type("SuggestKeywordThemesResponse") returns)
+    mock_suggest_keyword_themes_response_type = MagicMock()
+    mock_suggest_keyword_themes_response_type.KeywordTheme = mock_keyword_theme_constructor
+
+    # Mock for SmartCampaignSuggestionInfo
+    mock_smart_campaign_suggestion_info_instance = MagicMock()
+    mock_smart_campaign_suggestion_info_instance.location_list = MagicMock()
+    mock_smart_campaign_suggestion_info_instance.location_list.locations = [] # Must be a list for .append()
+    mock_smart_campaign_suggestion_info_instance.ad_schedules = [] # Must be a list for .append()
+
+    # Mock for LocationInfo and AdScheduleInfo (simple mocks are fine)
+    mock_location_info_instance = MagicMock()
+    mock_ad_schedule_info_instance = MagicMock()
+
+    def new_get_type_side_effect(type_name):
         if type_name == "SuggestKeywordThemesResponse":
-            return mock_suggest_response_type_with_keyword_theme_attr
-        # Fallback to the default behavior of the original get_type mock for other types
-        if hasattr(original_get_type_mock, 'side_effect') and original_get_type_mock.side_effect:
-            # If the original mock itself had a side_effect
-            return original_get_type_mock.side_effect(type_name)
-        return original_get_type_mock(type_name)
-
-    mock_google_ads_client.get_type.side_effect = get_type_side_effect_for_keyword_theme
+            return mock_suggest_keyword_themes_response_type
+        elif type_name == "SmartCampaignSuggestionInfo":
+            # Important: return the instance directly if the script does client.get_type("SmartCampaignSuggestionInfo")
+            # and expects to set attributes on it. If it calls it like a constructor, then this should return a mock constructor.
+            # The script does: suggestion_info = client.get_type("SmartCampaignSuggestionInfo")
+            # then suggestion_info.location_list = ...
+            # So, returning the instance directly is correct.
+            return mock_smart_campaign_suggestion_info_instance
+        elif type_name == "LocationInfo":
+            # Script does: location = client.get_type("LocationInfo")
+            return mock_location_info_instance
+        elif type_name == "AdScheduleInfo":
+            # Script does: ad_schedule = client.get_type("AdScheduleInfo")
+            return mock_ad_schedule_info_instance
+        else:
+            # Fallback for any other type
+            return MagicMock()
+
+    mock_google_ads_client.get_type.side_effect = new_get_type_side_effect
     # --- End of get_type mocking ---
 
     # Suggest Budget Options
@@ -238,40 +253,38 @@ def test_main_with_business_location_runs_successfully(mock_uuid4_biz: MagicMock
     mock_empty_kw_theme_response.keyword_themes = [] # No themes from this service for this test path
     mock_suggest_service.suggest_keyword_themes.return_value = mock_empty_kw_theme_response
 
-    # --- Mocking client.get_type for KeywordTheme instantiation (same as in the first test) ---
-    def create_mock_keyword_theme_instance_biz():
-        instance = MagicMock(spec=['keyword_theme_constant', 'free_form_keyword_theme'])
-        instance.keyword_theme_constant = None
-        instance.free_form_keyword_theme = None
-        return instance
-    
-    mock_keyword_theme_constructor_biz = MagicMock(side_effect=create_mock_keyword_theme_instance_biz)
-    mock_suggest_response_type_with_keyword_theme_attr_biz = MagicMock()
-    mock_suggest_response_type_with_keyword_theme_attr_biz.KeywordTheme = mock_keyword_theme_constructor_biz
+    # --- Mocking client.get_type for various types (similar to the first test) ---
+    mock_google_ads_client.get_type = MagicMock() # Reset to a simple mock
+
+    mock_keyword_theme_instance_biz = MagicMock(spec=['keyword_theme_constant', 'free_form_keyword_theme'])
+    mock_keyword_theme_instance_biz.keyword_theme_constant = None
+    mock_keyword_theme_instance_biz.free_form_keyword_theme = None
+    mock_keyword_theme_constructor_biz = MagicMock(return_value=mock_keyword_theme_instance_biz)
+
+    mock_suggest_keyword_themes_response_type_biz = MagicMock()
+    mock_suggest_keyword_themes_response_type_biz.KeywordTheme = mock_keyword_theme_constructor_biz
+
+    mock_smart_campaign_suggestion_info_instance_biz = MagicMock()
+    mock_smart_campaign_suggestion_info_instance_biz.location_list = MagicMock()
+    mock_smart_campaign_suggestion_info_instance_biz.location_list.locations = []
+    mock_smart_campaign_suggestion_info_instance_biz.ad_schedules = []
 
-    original_get_type_mock_biz = mock_google_ads_client.get_type # already a side_effect from previous test
-                                                                # or the conftest default.
-                                                                # We need to be careful not to infinitely recurse if tests run in same session
-                                                                # But the test runner should give fresh client for each test.
-
-    # Re-assigning side_effect for this test case
-    # Note: If the mock_google_ads_client is shared and mutated across tests, this could be an issue.
-    # Pytest fixtures usually provide a fresh mock per test.
-    current_get_type_behavior = mock_google_ads_client.get_type 
-    # If current_get_type_behavior is already our complex side_effect from the *same* test pass (it shouldn't be),
-    # we'd need to access the 'original' it captured. But pytest should prevent this.
-
-    def get_type_side_effect_for_keyword_theme_biz(type_name):
+    mock_location_info_instance_biz = MagicMock()
+    mock_ad_schedule_info_instance_biz = MagicMock()
+
+    def new_get_type_side_effect_biz(type_name):
         if type_name == "SuggestKeywordThemesResponse":
-            return mock_suggest_response_type_with_keyword_theme_attr_biz
-        # Fallback for other types
-        if hasattr(current_get_type_behavior, "__call__") and not isinstance(current_get_type_behavior, MagicMock):
-             # If it's a function (like a previous side_effect)
-            return current_get_type_behavior(type_name)
-        return current_get_type_behavior # If it's a simple MagicMock
-        
-    mock_google_ads_client.get_type = MagicMock() # Reset to a simple mock first
-    mock_google_ads_client.get_type.side_effect = get_type_side_effect_for_keyword_theme_biz # Then apply new side_effect
+            return mock_suggest_keyword_themes_response_type_biz
+        elif type_name == "SmartCampaignSuggestionInfo":
+            return mock_smart_campaign_suggestion_info_instance_biz
+        elif type_name == "LocationInfo":
+            return mock_location_info_instance_biz
+        elif type_name == "AdScheduleInfo":
+            return mock_ad_schedule_info_instance_biz
+        else:
+            return MagicMock()
+
+    mock_google_ads_client.get_type.side_effect = new_get_type_side_effect_biz
     # --- End of get_type mocking for biz test ---
 
     mock_budget_options_response = MagicMock()

examples/advanced_operations/tests/test_get_ad_group_bid_modifiers.py

@@ -3,42 +3,109 @@
 
 from examples.advanced_operations.get_ad_group_bid_modifiers import main
 
+# --- Helper Mock Class ---
+class MockAdGroupBidModifierModel:
+    def __init__(self, client_enums):
+        # Attributes accessed directly by the script's print statement
+        self.criterion_id = MagicMock(name="criterion_id")
+        self.bid_modifier = MagicMock(name="bid_modifier")
+        
+        self.device = MagicMock(name="device_criterion")
+        # Ensure type_ itself can have .name called on it
+        self.device.type_ = MagicMock(name="device_type_enum_value")
+        self.device.type_.name = "UNKNOWN_DEVICE" # Default .name for the enum value
+
+        # Hotel criteria attributes
+        self.hotel_date_selection_type = MagicMock(name="hotel_date_selection_type_criterion")
+        self.hotel_date_selection_type.type_ = MagicMock(name="hotel_date_selection_type_enum_value")
+        self.hotel_date_selection_type.type_.name = "UNKNOWN_HOTEL_DATE_SELECTION"
+
+        self.hotel_advance_booking_window = MagicMock(name="hotel_advance_booking_window_criterion")
+        self.hotel_advance_booking_window.min_days = None
+        self.hotel_advance_booking_window.max_days = None
+
+        self.hotel_length_of_stay = MagicMock(name="hotel_length_of_stay_criterion")
+        self.hotel_length_of_stay.min_nights = None
+        self.hotel_length_of_stay.max_nights = None
+
+        self.hotel_check_in_day = MagicMock(name="hotel_check_in_day_criterion")
+        self.hotel_check_in_day.day_of_week = MagicMock(name="day_of_week_enum_value")
+        self.hotel_check_in_day.day_of_week.name = "UNSPECIFIED_DAY"
+        
+        self.hotel_check_in_date_range = MagicMock(name="hotel_check_in_date_range_criterion")
+        self.hotel_check_in_date_range.start_date = None
+        self.hotel_check_in_date_range.end_date = None
+        
+        # This will store the name of the 'oneof' criterion field (e.g., "device")
+        self._active_criterion_field = None # Default to None, must be set by test
+
+    @classmethod
+    def pb(cls, instance_self):
+        mock_pb_message = MagicMock(name="pb_message")
+        mock_pb_message.WhichOneof.return_value = instance_self._active_criterion_field
+        return mock_pb_message
+
+    def set_active_criterion_field(self, field_name):
+        self._active_criterion_field = field_name
+        # Basic way to somewhat mimic oneof: clear other fields if one is set.
+        # More robust would be to ensure only the active field has a non-default/non-None value.
+        # For this mock, primarily WhichOneof matters for the script's logic.
+        all_criteria_fields = [
+            "device", "hotel_date_selection_type", "hotel_advance_booking_window",
+            "hotel_length_of_stay", "hotel_check_in_day", "hotel_check_in_date_range"
+        ]
+        if field_name not in all_criteria_fields and field_name is not None:
+            raise ValueError(f"Unknown criterion field: {field_name}")
+        return self
+
+# --- Test Functions ---
 def test_main_runs_successfully(mock_google_ads_client: MagicMock) -> None:
     """Tests that the main function runs without raising an exception with an ad_group_id."""
     mock_customer_id = "123"
     mock_ad_group_id = "456"
 
-    # Mock GoogleAdsService for search
+    # --- Mock Enums (ensure they have .name attribute) ---
+    mock_google_ads_client.enums.DeviceEnum.MOBILE.name = "MOBILE_DEVICE"
+    mock_google_ads_client.enums.DeviceEnum.TABLET.name = "TABLET_DEVICE" # Just in case
+    mock_google_ads_client.enums.DeviceEnum.DESKTOP.name = "DESKTOP_DEVICE" # Just in case
+    mock_google_ads_client.enums.DeviceEnum.UNKNOWN.name = "UNKNOWN_DEVICE" # Default
+
+    mock_google_ads_client.enums.DayOfWeekEnum.MONDAY.name = "MONDAY"
+    mock_google_ads_client.enums.DayOfWeekEnum.UNSPECIFIED.name = "UNSPECIFIED_DOW" # Default
+    
+    # Add other hotel enums if specific paths are tested, e.g., HotelDateSelectionTypeEnum
+    mock_google_ads_client.enums.HotelDateSelectionTypeEnum.DEFAULT_SELECTION.name = "DEFAULT_SELECTION"
+    mock_google_ads_client.enums.HotelDateSelectionTypeEnum.UNKNOWN.name = "UNKNOWN_HDST" # Default
+
+    # --- Mock GoogleAdsService for search ---
     mock_googleads_service = mock_google_ads_client.get_service("GoogleAdsService")
 
-    mock_search_response_page = MagicMock() # Represents one page of results
+    # Row 1: Device Modifier
     mock_row1 = MagicMock()
-    mock_row1.ad_group_bid_modifier.criterion_id = 12345
-    mock_row1.ad_group_bid_modifier.bid_modifier = 1.5
-    mock_row1.ad_group_bid_modifier.device.type_ = mock_google_ads_client.enums.DeviceEnum.MOBILE
+    modifier1 = MockAdGroupBidModifierModel(mock_google_ads_client.enums)
+    modifier1.criterion_id = "1001"
+    modifier1.bid_modifier = 1.5
+    modifier1.set_active_criterion_field("device")
+    modifier1.device.type_ = mock_google_ads_client.enums.DeviceEnum.MOBILE
+    mock_row1.ad_group_bid_modifier = modifier1
     mock_row1.ad_group.id = int(mock_ad_group_id)
-    mock_row1.campaign.id = 789
-
-    mock_row2 = MagicMock() # Example for a non-device modifier (e.g. hotel)
-    mock_row2.ad_group_bid_modifier.criterion_id = 67890
-    mock_row2.ad_group_bid_modifier.bid_modifier = 0.8
-    # For hotel check-in day, the device field won't be populated.
-    # Instead, hotel_check_in_day field would be, for example:
-    mock_row2.ad_group_bid_modifier.hotel_check_in_day.day_of_week = mock_google_ads_client.enums.DayOfWeekEnum.MONDAY
-    # Clear other oneof fields like device for this specific row if the script checks for their absence
-    delattr(mock_row2.ad_group_bid_modifier, 'device') 
+    mock_row1.campaign.id = "789"
+
+    # Row 2: Hotel Check-in Day Modifier
+    mock_row2 = MagicMock()
+    modifier2 = MockAdGroupBidModifierModel(mock_google_ads_client.enums)
+    modifier2.criterion_id = "2002"
+    modifier2.bid_modifier = 0.8
+    modifier2.set_active_criterion_field("hotel_check_in_day")
+    modifier2.hotel_check_in_day.day_of_week = mock_google_ads_client.enums.DayOfWeekEnum.MONDAY
+    mock_row2.ad_group_bid_modifier = modifier2
     mock_row2.ad_group.id = int(mock_ad_group_id)
-    mock_row2.campaign.id = 789
+    mock_row2.campaign.id = "790"
 
+    mock_search_response_page = MagicMock()
     mock_search_response_page.results = [mock_row1, mock_row2]
-    # The search method returns an iterable of pages (MagicMock with results)
     mock_googleads_service.search.return_value = iter([mock_search_response_page])
 
-    # Ensure enums used in results and script are available (DeviceEnum, DayOfWeekEnum)
-    # These should be on mock_google_ads_client.enums from conftest.
-    # e.g. mock_google_ads_client.enums.DeviceEnum.MOBILE
-    # e.g. mock_google_ads_client.enums.DayOfWeekEnum.MONDAY
-
     try:
         main(
             mock_google_ads_client,
@@ -51,18 +118,28 @@ def test_main_runs_successfully(mock_google_ads_client: MagicMock) -> None:
 def test_main_runs_without_ad_group_id(mock_google_ads_client: MagicMock) -> None:
     """Tests that the main function runs without an ad_group_id (fetches for all ad groups)."""
     mock_customer_id = "123"
-    mock_ad_group_id = None
+    mock_ad_group_id = None # Key difference for this test
+
+    # --- Mock Enums (ensure they have .name attribute) ---
+    # Ensure these are available on the mock_google_ads_client.enums object
+    # The conftest should provide the base enums, we just need to ensure .name is on values.
+    mock_google_ads_client.enums.DeviceEnum.TABLET.name = "TABLET_DEVICE"
+    mock_google_ads_client.enums.DeviceEnum.UNKNOWN.name = "UNKNOWN_DEVICE"
 
+    # --- Mock GoogleAdsService for search ---
     mock_googleads_service = mock_google_ads_client.get_service("GoogleAdsService")
 
-    mock_search_response_page = MagicMock()
     mock_row1 = MagicMock()
-    mock_row1.ad_group_bid_modifier.criterion_id = 54321
-    mock_row1.ad_group_bid_modifier.bid_modifier = 1.2
-    mock_row1.ad_group_bid_modifier.device.type_ = mock_google_ads_client.enums.DeviceEnum.TABLET
-    mock_row1.ad_group.id = 987 # Different ad group ID
-    mock_row1.campaign.id = 654
+    modifier1 = MockAdGroupBidModifierModel(mock_google_ads_client.enums)
+    modifier1.criterion_id = "3003"
+    modifier1.bid_modifier = 1.2
+    modifier1.set_active_criterion_field("device")
+    modifier1.device.type_ = mock_google_ads_client.enums.DeviceEnum.TABLET
+    mock_row1.ad_group_bid_modifier = modifier1
+    mock_row1.ad_group.id = "987" # Different ad group ID
+    mock_row1.campaign.id = "654"
 
+    mock_search_response_page = MagicMock()
     mock_search_response_page.results = [mock_row1]
     mock_googleads_service.search.return_value = iter([mock_search_response_page])