Merge pull request #650 from mozilla/philimon/normalize_regional_phone_fix

Philimon/Normalize_Telephone_Numbers

Author: philimon-reset

.github/workflows/l10n.yml

@@ -3,7 +3,6 @@ name: L10N Test Execution
 
 run-name: ${{ github.actor }} is running l10n tests
 on:
-  pull_request:
   workflow_call:
     inputs:
       channel:

ci_l10n_pyproject_headed.toml

@@ -15,7 +15,7 @@ markers = [
     "noxvfb: tests that should not run in xvfb sessions"
 ]
 
-addopts = "-vs --ci --json-report --json-report-file artifacts/report_headed.json -n auto --reruns 3 --reruns-delay 2 --html=artifacts/report_headed.html"
+addopts = "-vs --ci --json-report --json-report-file artifacts/report_headed.json -n auto --reruns 4 --reruns-delay 3 --html=artifacts/report_headed.html"
 
 [tool.ruff]
 target-version = "py310"

l10n_CM/Unified/test_demo_ad_0_add_new_address.py

@@ -52,7 +52,7 @@ def test_verify_new_address_is_added(
 def data_sanitizer(util: Utilities, value: str, region: str, state_province: Dict):
     value = value.strip()
     if value[0] == "+":
-        return util.normalize_phone_number(value)
+        return util.normalize_regional_phone_numbers(value, region)
     elif len(value) == 2 and value != region:
         return state_province.get(value, value)
     return value

l10n_CM/region/US.json

@@ -6,6 +6,7 @@
     "lowes",
     "etsy",
     "calvinklein",
+    "bestbuy",
     "demo"
   ],
   "tests": []

modules/page_object_autofill.py

@@ -190,6 +190,7 @@ def update_form_data(
         sample_data: AutofillAddressBase | CreditCardBase,
         field: str,
         value: str | int,
+        region: str,
     ):
         """
         Update the form field with the new value.
@@ -198,6 +199,7 @@ def update_form_data(
             sample_data: sample data instance used to verify change.
             field: field being changed.
             value: value being added.
+            region: region being tested.
         """
         # updating the profile accordingly
         self.update_and_save(field, value)
@@ -206,10 +208,12 @@ def update_form_data(
         self.select_autofill_option(field)
 
         # verifying the correct data
-        self.verify_form_data(sample_data)
+        self.verify_form_data(sample_data, region)
         return self
 
-    def verify_form_data(self, sample_data: CreditCardBase | AutofillAddressBase):
+    def verify_form_data(
+        self, sample_data: CreditCardBase | AutofillAddressBase, region: str = "US"
+    ):
         """Verify that form is filled correctly against sample data."""
         if not self.field_mapping:
             # Method is meant to be called by one of the classes that inherit AutoFill (CreditCardFill or AddressFill)
@@ -229,34 +233,55 @@ def verify_form_data(self, sample_data: CreditCardBase | AutofillAddressBase):
             if attr_name == "cvv":
                 continue
             expected_value = getattr(sample_data, attr_name, None)
-            autofilled_field = self.get_element("form-field", labels=[field_name])
-            if autofilled_field.tag_name.lower() != "select":
-                autofilled_field_value = autofilled_field.get_attribute("value")
-            else:
-                autofilled_field_value = Select(
-                    autofilled_field
-                ).first_selected_option.text
-            if (
-                attr_name == "address_level_1"
-                and autofilled_field_value != expected_value
-            ):
-                expected_value = self.util.get_state_province_abbreviation(
-                    expected_value
-                )
-            elif attr_name == "expiration_date" and len(autofilled_field_value) > 5:
-                autofilled_field_value = autofilled_field_value.replace("20", "")
-
-            elif attr_name == "country":
-                expected_value = self.util.get_country_local_translation(expected_value)
-            if attr_name == "expiration_month" and autofilled_field_value.isdigit():
-                # Handle expiration month comparison - normalize to integers to handle leading zeros
-                autofilled_field_value = str(int(autofilled_field_value))
-                expected_value = str(int(expected_value))
-
-            assert expected_value in autofilled_field_value, (
-                f"{autofilled_field_value} is different from {expected_value}"
+            auto_filled_field_value = self._get_field_value(field_name)
+
+            # Normalize values for comparison
+            expected_value, auto_filled_field_value = self._normalize_values(
+                attr_name, expected_value, auto_filled_field_value, region
+            )
+
+            assert expected_value in auto_filled_field_value, (
+                f"Field '{attr_name}' ('{field_name}'): expected '{expected_value}' to be in '{auto_filled_field_value}'"
             )
 
+    def _get_field_value(self, field_name: str) -> str:
+        """Get the value from a form field, handling different element types."""
+        autofilled_field = self.get_element("form-field", labels=[field_name])
+        if autofilled_field.tag_name.lower() != "select":
+            return autofilled_field.get_attribute("value")
+        else:
+            return Select(autofilled_field).first_selected_option.text
+
+    def _normalize_values(
+        self,
+        attr_name: str,
+        expected_value: str,
+        auto_filled_field_value: str,
+        region: str,
+    ) -> tuple[str, str]:
+        """Normalize expected and actual values for comparison."""
+        if attr_name == "address_level_1" and auto_filled_field_value != expected_value:
+            expected_value = self.util.get_state_province_abbreviation(expected_value)
+        elif attr_name == "expiration_date" and len(auto_filled_field_value) > 5:
+            auto_filled_field_value = auto_filled_field_value.replace("20", "")
+        elif attr_name == "country":
+            expected_value = self.util.get_country_local_translation(expected_value)
+        elif attr_name == "telephone":
+            expected_value = self.util.normalize_regional_phone_numbers(
+                expected_value, region
+            )
+            auto_filled_field_value = self.util.normalize_regional_phone_numbers(
+                auto_filled_field_value, region
+            )
+
+        # Handle numeric fields
+        if attr_name == "expiration_month" and auto_filled_field_value.isdigit():
+            # Handle expiration month comparison - normalize to integers to handle leading zeros
+            auto_filled_field_value = str(int(auto_filled_field_value))
+            expected_value = str(int(expected_value))
+
+        return expected_value, auto_filled_field_value
+
     def verify_field_autofill_dropdown(
         self,
         fields_to_test: List[str] = None,
@@ -433,7 +458,7 @@ def verify_all_fields_cleared(self):
 
     @BasePage.context_chrome
     def verify_autofill_data_on_hover(
-        self, autofill_data: CreditCardBase | AutofillAddressBase
+        self, autofill_data: CreditCardBase | AutofillAddressBase, region: str
     ):
         """
         Verifies that the autofill preview data matches the expected values when hovering
@@ -473,7 +498,7 @@ def verify_autofill_data_on_hover(
                 )
         for field, value in container_data.items():
             if field in self.preview_fields:
-                value = self.sanitize_preview_data(field, str(value))
+                value = self.sanitize_preview_data(field, str(value), region)
                 # Check if this value exists in our CreditCardBase | AutofillAddressBase object
                 is_present = any(
                     [value in val for val in autofill_data.__dict__.values()]
@@ -482,13 +507,13 @@ def verify_autofill_data_on_hover(
                     f"Mismatched data: {(field, value)} not in {autofill_data.__dict__.values()}."
                 )
 
-    def sanitize_preview_data(self, field, value):
+    def sanitize_preview_data(self, field, value, region):
         if field == "cc-number":
             value = value[-4:]
         elif field == "cc-exp-year":
             value = value[-2:]
         elif field == "tel" or value[0] == "+":
-            value = self.util.normalize_phone_number(value)
+            value = self.util.normalize_regional_phone_numbers(value, region)
         return value
 
     def select_autofill_option(self, field, index: int = 1):
@@ -576,7 +601,7 @@ def check_autofill_preview_for_field(
                 self.double_click("form-field", labels=[field])
                 self.autofill_popup.ensure_autofill_dropdown_visible()
                 self.autofill_popup.hover("select-form-option")
-                self.verify_autofill_data_on_hover(sample_data)
+                self.verify_autofill_data_on_hover(sample_data, region)
                 self.click_on("form-field", labels=[field])
             else:
                 logging.info(
@@ -636,7 +661,7 @@ def clear_and_verify(
 
                 if sample_data:
                     ## verify data
-                    self.verify_form_data(sample_data)
+                    self.verify_form_data(sample_data, region)
 
                 # Clear form autofill
                 self.double_click("form-field", labels=[field])

modules/util.py

@@ -264,7 +264,9 @@ def generate_localized_phone(self, country_code: str, fake: Faker) -> str:
                 if phone[:2] != "11":
                     break
         else:
-            phone = self.normalize_phone_number(fake.phone_number())
+            phone = self.normalize_regional_phone_numbers(
+                fake.phone_number(), country_code
+            )
         return phone
 
     def fake_autofill_data(self, country_code) -> AutofillAddressBase:
@@ -517,9 +519,9 @@ def get_state_province_abbreviation(self, full_name: str) -> str:
         Returns the abbreviation for a given state, province, or region full name.
 
         :param full_name: The full name of the state, province, or region.
-        :return: The corresponding abbreviation or "Not Found" if not in the dictionary.
+        :return: The corresponding abbreviation or the full name itself if not in the dictionary.
         """
-        return self.state_province_abbr.get(full_name, "Not Found")
+        return self.state_province_abbr.get(full_name, full_name)
 
     def get_country_local_translation(self, country_name: str) -> str:
         """
@@ -562,16 +564,14 @@ def normalize_regional_phone_numbers(self, phone: str, region: str) -> str:
         # Determine country code
         country_code = country_codes.get(
             region, "1"
-        )  # Default to "1" (US/CA) if region is unknown
+        )  # Default to "1" (US/CA) if the region is unknown
+        # handle leading zeros
         local_number = digits
 
-        # Check if phone already contains a valid country code
-        for code in country_codes.values():
-            if digits.startswith(code):
-                country_code = code
-                # Remove country code from local number
-                local_number = digits[len(code) :]
-                break
+        # Check if the phone number already contains a valid country code
+        if digits.startswith(country_code):
+            # Remove country code from the local number
+            local_number = digits[len(country_code) :]
 
         # Handle leading zero in local numbers (France & Germany)
         if region in ["FR", "DE"] and local_number.startswith("0"):

tests/form_autofill/test_updating_address.py

@@ -44,7 +44,7 @@ def test_update_address(
     # Change name field value.
     new_name_value = sample_data.name + " Doe"
     setattr(sample_data, "name", new_name_value)
-    address_autofill.update_form_data(sample_data, "name", new_name_value)
+    address_autofill.update_form_data(sample_data, "name", new_name_value, region)
 
     # Navigate to settings
     about_prefs_privacy.open()

tests/form_autofill/test_updating_credit_card.py

@@ -57,7 +57,7 @@ def test_update_cc_no_dupe_name(
         credit_card_sample_data, field, region
     )
     credit_card_autofill.update_form_data(
-        credit_card_sample_data, field, new_field_value
+        credit_card_sample_data, field, new_field_value, region
     )
 
     # navigate to settings