Repository: SFDO-Tooling/CumulusCI
Pull request #3366 (2022-09-20 23:19:22)

Fixes for failing tests with firefox
This PR is for work item [W-11596166](https://gus.lightning.force.com/a07EE0000148IXIYA2), for preventing errors related to cross-domain cookies.  It also fixes a couple of small issues related to firefox.  Specifically, this includes a fix so that if we fail to take a screenshot (which only seems to happen on firefox) we don't let that failure break the test or hide another error that caused the screenshot to be taken.

Finally, there are a couple of other small changes related to test instability.


# Critical Changes

# Changes
* A failure when calling `capture page screenshot` no longer causes a robot test to fail
* For robot tests on firefox, the firefox profile now accepts all cookies to prevent lightning pages from failing to render due to cross-domain cookies. 
* We've improved the method that playwright-based tests use to wait for the browser to initialize

# Issues Closed


Patch:
diff --git a/cumulusci/robotframework/Salesforce.robot b/cumulusci/robotframework/Salesforce.robot
index ac51fb275..1b8e9275f 100644
--- a/cumulusci/robotframework/Salesforce.robot
+++ b/cumulusci/robotframework/Salesforce.robot
@@ -134,17 +134,25 @@ Open Test Browser Firefox
     [Documentation]  Opens a Firefox browser window and navigates to the org
     ...  This keyword isn't normally called directly by a test. It is used
     ...  by the `Open Test Browser` keyword.
+    ...
+    ...  The firefox profile is set to accept all cookies.
 
     [Arguments]     ${login_url}  ${alias}=${NONE}
     Open Browser  ${login_url}  firefox  alias=${alias}
+    #    http://kb.mozillazine.org/Network.cookie.cookieBehavior
+    ...  ff_profile_dir=set_preference("network.cookie.cookieBehavior", 0)
 
 Open Test Browser Headless Firefox
     [Documentation]  Opens the firefox browser in headless mode
     ...  This keyword isn't normally called directly by a test. It is used
     ...  by the `Open Test Browser` keyword.
+    ...
+    ...  The firefox profile is set to accept all cookies.
 
     [Arguments]     ${login_url}  ${alias}=${NONE}
     Open Browser  ${login_url}  headlessfirefox  alias=${alias}
+    #    http://kb.mozillazine.org/Network.cookie.cookieBehavior
+    ...  ff_profile_dir=set_preference("network.cookie.cookieBehavior", 0)
 
 Get Chrome Options
     [Documentation]
diff --git a/cumulusci/robotframework/SalesforcePlaywright.py b/cumulusci/robotframework/SalesforcePlaywright.py
index d5663b424..3406c3865 100644
--- a/cumulusci/robotframework/SalesforcePlaywright.py
+++ b/cumulusci/robotframework/SalesforcePlaywright.py
@@ -4,7 +4,10 @@ from Browser import SupportedBrowsers
 
 from cumulusci.robotframework.base_library import BaseLibrary
 from cumulusci.robotframework.faker_mixin import FakerMixin
-from cumulusci.robotframework.utils import WAIT_FOR_AURA_SCRIPT
+from cumulusci.robotframework.utils import (
+    WAIT_FOR_AURA_SCRIPT,
+    capture_screenshot_on_error,
+)
 
 
 class SalesforcePlaywright(FakerMixin, BaseLibrary):
@@ -88,10 +91,6 @@ class SalesforcePlaywright(FakerMixin, BaseLibrary):
             else self.cumulusci.login_url()
         )
 
-        # browser's (or robot's?) automatic type conversion doesn't
-        # seem to work when calling the function directly, so we have
-        # to pass the enum rather than string representation of the
-        # browser. _sigh_
         if record_video:
             # ugh. the "dir" value must be non-empty, and will be treated as
             # a folder name under the browser/video folder. using "../video"
@@ -106,10 +105,10 @@ class SalesforcePlaywright(FakerMixin, BaseLibrary):
         )
         page_details = self.browser.new_page(login_url)
 
-        self.browser.wait_until_network_is_idle()
-
+        self.wait_until_loading_is_complete()
         return browser_id, context_id, page_details
 
+    @capture_screenshot_on_error
     def wait_until_loading_is_complete(self, locator=None):
         """Wait for a lightning page to load.
 
@@ -127,14 +126,14 @@ class SalesforcePlaywright(FakerMixin, BaseLibrary):
             if locator is None
             else locator
         )
+        self.browser.get_element(locator)
+        self.browser.execute_javascript(function=WAIT_FOR_AURA_SCRIPT)
+
+        # this seems to fail once in a while for an unknown reason, so we'll
+        # try twice. This seems to be more effective than calling the
+        # function once and waiting 30 seconds.
         try:
-            self.browser.get_element(locator)
-            self.browser.execute_javascript(function=WAIT_FOR_AURA_SCRIPT)
-            self.browser.wait_until_network_is_idle()
-
-        except Exception:
-            try:
-                self.browser.take_screenshot()
-            except Exception as e:
-                self.builtin.warn("unable to capture screenshot: {}".format(str(e)))
-            raise
+            self.browser.wait_until_network_is_idle("15 seconds")
+        except Exception as e:
+            self.builtin.log(f"caught error waiting for idle: {e}", "WARN")
+            self.browser.wait_until_network_is_idle("15 seconds")
diff --git a/cumulusci/robotframework/form_handlers.py b/cumulusci/robotframework/form_handlers.py
index 92f0791cd..ceabfcb39 100644
--- a/cumulusci/robotframework/form_handlers.py
+++ b/cumulusci/robotframework/form_handlers.py
@@ -194,18 +194,16 @@ class LightningLookupHandler(BaseFormHandler):
     tags = ["lightning-lookup", "lightning-grouped-combobox"]
 
     def set(self, value):
-        wait = 5
+        wait = 10
         # I wonder if I should/could anchor this to the element
         # instead of searching the whole document?
-        value_locator = f'//div[@role="listbox"]//*[.="{value}"]'
+        value_locator = f'//lightning-base-combobox-formatted-text[@title="{value}"]'
         self.element.click()
         self.input_element.send_keys(value)
         try:
             self.selenium.wait_until_element_is_visible(value_locator, wait)
             self.selenium.click_element(value_locator)
         except Exception:
-            # *sigh* this still fails randomly even though
-            # I can see the dadgum item in the dropdown.
             raise TimeoutException(
                 f"Lookup value '{value}' for '{self.locator}' not found after {wait} seconds"
             )
diff --git a/cumulusci/robotframework/utils.py b/cumulusci/robotframework/utils.py
index a0cdf4d3d..703088810 100644
--- a/cumulusci/robotframework/utils.py
+++ b/cumulusci/robotframework/utils.py
@@ -95,7 +95,7 @@ class RetryingSeleniumLibraryMixin(object):
                         result = orig_execute(driver_command, params)
                 except Exception:
                     if driver_command != Command.SCREENSHOT:
-                        self.selenium.capture_page_screenshot()
+                        safe_screenshot()
                     if self.debug:
                         self.selenium.log_source()
                     raise
@@ -212,17 +212,17 @@ def selenium_retry(target=None, retry=True):
 def capture_screenshot_on_error(func):
     """Decorator for capturing a screenshot if a keyword throws an error
 
-    The name is slightly misleading. While it was designed to capture a
-    screenshot on error, it actually ties in to the same feature in
-    SeleniumLibrary.
+    This decorator will work for both SeleniumLibrary and Browser.
 
-    SeleniumLibrary lets you define any keyword to be run on error,
-    though I doubt we would ever use it for anything other than to
-    capture screenshots. However, if you do configure SeleniumLibrary
-    to do something other than (or in addition to) capturing a
-    screenshot, any keyword that uses this decorator will behave just
-    like a SeleniumLibrary keyword.
+    If a screenshot cannot be taken for any reason, the screenshot error
+    will be logged before the original exception is re-raised.
 
+    For keywords using the Browser library, this will call the keyword
+    "Take screenshot".
+
+    For keywords using SeleniumLibrary, this decorator will call whatever
+    keyword SeleniumLibrary is configured to call on failure. Normally
+    that will be "Capture page screenshot".
     """
 
     @functools.wraps(func)
@@ -230,13 +230,28 @@ def capture_screenshot_on_error(func):
         try:
             func(*args, **kwargs)
         except Exception:
-            selib = BuiltIn().get_library_instance("SeleniumLibrary")
-            selib.failure_occurred()
+            safe_screenshot()
             raise
 
     return wrapper
 
 
+def safe_screenshot():
+    """Take a screenshot, but log and then ignore errors if the screenshot fails
+
+    This function should work for both selenium and browser (playwright) libraries
+    """
+
+    libs = BuiltIn().get_library_instance(all=True)
+    try:
+        if "Browser" in libs:
+            libs["Browser"].take_screenshot()
+        elif "SeleniumLibrary" in libs:
+            libs["SeleniumLibrary"].capture_page_screenshot()
+    except Exception as e:
+        BuiltIn().log(f"unable to capture screenshot: {e}")
+
+
 def get_locator_module_name(version):
     """Return module name of locator file for the specified version
 


Test patch:
diff --git a/cumulusci/robotframework/tests/salesforce/forms.robot b/cumulusci/robotframework/tests/salesforce/forms.robot
index 912173107..41167aff7 100644
--- a/cumulusci/robotframework/tests/salesforce/forms.robot
+++ b/cumulusci/robotframework/tests/salesforce/forms.robot
@@ -124,9 +124,10 @@ Non-lightning based form - radiobutton
     [Documentation]  Verify we can set a plain non-lightning radiobutton
 
     [Setup]     Run keywords
-    ...  Skip if  "firefox" in $browser
-    ...  AND  Go to My Email Settings
-    ...  AND  Select frame  //div[@class="setupcontent"]//iframe
+    ...  Go to My Email Settings
+    ...  AND  Select frame                   //div[@class="setupcontent"]//iframe
+    ...  AND  Select radio button            use_external_email  1
+    ...  AND  Radio button should be set to  use_external_email  1
     [Teardown]  Unselect frame
 
     # The settings page is just about the only page I could find
@@ -134,9 +135,6 @@ Non-lightning based form - radiobutton
     # Thankfully, I can use built-in keywords to validate that
     # the radiobuttons have actually bet set.
 
-    # make sure it is set to 1
-    Select radio button  use_external_email  1
-
     # then try to use our keyword to set it
     Input form data
     ...  Send through Salesforce  selected
diff --git a/cumulusci/robotframework/tests/salesforce/pageobjects/listing_page.robot b/cumulusci/robotframework/tests/salesforce/pageobjects/listing_page.robot
index de86640ba..c83a890fa 100644
--- a/cumulusci/robotframework/tests/salesforce/pageobjects/listing_page.robot
+++ b/cumulusci/robotframework/tests/salesforce/pageobjects/listing_page.robot
@@ -14,7 +14,13 @@ Create Test Data
     [Documentation]
     ...  Create contacts used by tests in this suite
 
-    # First, delete all existing contacts
+    # Delete all cases, in case they are holding on to some contacts
+    ${records}=  Salesforce Query  Case
+    FOR  ${record}  IN  @{records}
+        Salesforce Delete  Case  ${record['Id']}
+    END
+
+    # Next, delete all existing contacts
     ${records}=  Salesforce Query  Contact
     FOR  ${record}  IN  @{records}
         Salesforce Delete  Contact  ${record['Id']}
diff --git a/cumulusci/robotframework/tests/salesforce/pageobjects/objectmanager.robot b/cumulusci/robotframework/tests/salesforce/pageobjects/objectmanager.robot
index e6044cdd1..b97e696af 100644
--- a/cumulusci/robotframework/tests/salesforce/pageobjects/objectmanager.robot
+++ b/cumulusci/robotframework/tests/salesforce/pageobjects/objectmanager.robot
@@ -38,7 +38,6 @@ Navigate To The Object Manager page For Specified Object
 Create Custom Lookup Field Using Object Manager
     [Documentation]     To test the ability of creating a custom lookup field using object manager and verify field got created
 
-    Skip if  "firefox" in $browser
     Create Custom Field In Object Manager
     ...                                                    Object=Contact
     ...                                                    Field_Type=Lookup
@@ -52,7 +51,6 @@ Create Custom Lookup Field Using Object Manager
 Create Custom Text Field Using Object Manager
     [Documentation]     To test the ability of creating a custom text field using object manager and verify field got created
 
-    Skip if  "firefox" in $browser
     Create Custom Field In Object Manager
     ...                                                    Object=Contact
     ...                                                    Field_Type=Text
@@ -66,7 +64,6 @@ Create Custom Text Field Using Object Manager
 Create Custom Currency Field Using Object Manager
     [Documentation]     To test the ability of creating a custom currency field and verify field got created
 
-    Skip if  "firefox" in $browser
     Create Custom Field In Object Manager
     ...                                                    Object=Account
     ...                                                    Field_Type=Currency
diff --git a/cumulusci/robotframework/tests/salesforce/playwright/playwright.robot b/cumulusci/robotframework/tests/salesforce/playwright/playwright.robot
index 243333451..d2adf527f 100644
--- a/cumulusci/robotframework/tests/salesforce/playwright/playwright.robot
+++ b/cumulusci/robotframework/tests/salesforce/playwright/playwright.robot
@@ -17,7 +17,7 @@ Go to user profile
 
 Go to contacts home
     Click            button:has-text("App Launcher")
-    Fill text        input[placeholder='Search apps and items...']  Contacts
+    Type text        input[placeholder='Search apps and items...']  Contacts
     Click            one-app-launcher-menu-item:has-text("Contacts")
 
     Wait until network is idle
diff --git a/cumulusci/robotframework/tests/test_utils.py b/cumulusci/robotframework/tests/test_utils.py
index 78ed0c840..72249e1f8 100644
--- a/cumulusci/robotframework/tests/test_utils.py
+++ b/cumulusci/robotframework/tests/test_utils.py
@@ -6,11 +6,21 @@ from unittest import mock
 import cumulusci.robotframework.utils as robot_utils
 from cumulusci.utils import touch
 
+mock_SeleniumLibrary = mock.Mock()
+
+
+class MockBuiltIn:
+    get_library_instance = mock.Mock(
+        return_value={"SeleniumLibrary": mock_SeleniumLibrary}
+    )
+
+
+robot_utils.BuiltIn = MockBuiltIn
+
 
 class TestRobotframeworkUtils:
     def setup_method(self):
-        robot_utils.BuiltIn = mock.Mock(name="BuiltIn")
-        self.mock_selib = robot_utils.BuiltIn().get_library_instance("SeleniumLibrary")
+        mock_SeleniumLibrary.reset_mock()
 
     def test_screenshot_decorator_fail(self):
         """Verify that the decorator will capture a screenshot on keyword failure"""
@@ -23,7 +33,7 @@ class TestRobotframeworkUtils:
             example_function()
         except Exception:
             pass
-        self.mock_selib.failure_occurred.assert_called_once()
+        mock_SeleniumLibrary.capture_page_screenshot.assert_called_once()
 
     def test_screenshot_decorator_pass(self):
         """Verify that decorator does NOT capture screenshot on keyword success"""
@@ -33,7 +43,8 @@ class TestRobotframeworkUtils:
             return True
 
         example_function()
-        self.mock_selib.failure_occurred.assert_not_called()
+
+        mock_SeleniumLibrary.capture_page_screenshot.assert_not_called()
 
 
 class TestGetLocatorModule: