From 07ecd396b0e3f9b60eeda2e82b5f2151243c3c38 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 22 Jul 2022 08:18:26 +0000 Subject: [PATCH] [ADD] core: error if a test finishes with a form being edited Since 54ea9564905f6c140b4892357657e763d21b4fa5 the form view has an "urgentSave" fallback when the page is unloaded (tab closed, page navigated away from, ...). In tours, this translates to new network requests being performed during the browser cleanup, possibly chaining further into more network events. Flag these tours as incorrect (by making them fail if we find a form in edition mode after receiving `"test successful"`). Adjust testing of http cases because I've added an empty line between the signal and the actual message for better readability on complex error messages. Also provide opt-out, as for some tours it's difficult to impossible to truly fix them: the `allow_end_on_form` class attribute can be set to `True` in order to disable the new behaviour. To implement this, update the browser runner receive the test class directly (rather than just the test class' name) for more introspection flexibility. Part-of: odoo/odoo#96517 --- odoo/addons/base/tests/test_http_case.py | 4 +- odoo/tests/common.py | 90 +++++++++++++++++++++--- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/odoo/addons/base/tests/test_http_case.py b/odoo/addons/base/tests/test_http_case.py index 89ebe99eb3d..b0323091a24 100644 --- a/odoo/addons/base/tests/test_http_case.py +++ b/odoo/addons/base/tests/test_http_case.py @@ -15,7 +15,7 @@ class TestHttpCase(HttpCase): with patch('odoo.tests.common.ChromeBrowser.take_screenshot', return_value=None): self.browser_js(url_path='about:blank', code=code) # second line must contains error message - self.assertEqual(error_catcher.exception.args[0].splitlines()[1], "test error message") + self.assertEqual(error_catcher.exception.args[0].splitlines()[-1], "test error message") self.assertEqual(len(log_catcher.output), 1) self.assertIn('test error message', log_catcher.output[0]) @@ -26,7 +26,7 @@ class TestHttpCase(HttpCase): with patch('odoo.tests.common.ChromeBrowser.take_screenshot', return_value=None): self.browser_js(url_path='about:blank', code=code) # second line must contains error message - self.assertEqual(error_catcher.exception.args[0].splitlines()[1:3], + self.assertEqual(error_catcher.exception.args[0].splitlines()[-2:], ['TypeError: test error message', ' at :1:15']) self.assertEqual(len(log_catcher.output), 1) self.assertIn('TypeError: test error message\n at :1:15', log_catcher.output[0]) diff --git a/odoo/tests/common.py b/odoo/tests/common.py index f68df2fea91..e30695e302a 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -882,13 +882,52 @@ class SingleTransactionCase(BaseCase): class ChromeBrowserException(Exception): pass +def fmap(future, map_fun): + """Maps a future's result through a callback. + + Resolves to the application of ``map_fun`` to the result of ``future``. + + .. warning:: this does *not* recursively resolve futures, if that's what + you need see :func:`fchain` + """ + fmap_future = Future() + @future.add_done_callback + def _(f): + try: + fmap_future.set_result(map_fun(f.result())) + except Exception as e: + fmap_future.set_exception(e) + return fmap_future + +def fchain(future, next_callback): + """Chains a future's result to a new future through a callback. + + Corresponds to the ``bind`` monadic operation (aka flatmap aka then... + kinda). + """ + new_future = Future() + @future.add_done_callback + def _(f): + try: + n = next_callback(f.result()) + @n.add_done_callback + def _(f): + try: + new_future.set_result(f.result()) + except Exception as e: + new_future.set_exception(e) + + except Exception as e: + new_future.set_exception(e) + + return new_future class ChromeBrowser: """ Helper object to control a Chrome headless process. """ remote_debugging_port = 0 # 9222, change it in a non-git-tracked file - def __init__(self, logger, window_size, touch_enabled, test_class): - self._logger = logger + def __init__(self, test_class): + self._logger = test_class._logger self.test_class = test_class if websocket is None: self._logger.warning("websocket-client module is not installed") @@ -910,8 +949,8 @@ class ChromeBrowser: self.screencast_frames = [] os.makedirs(self.screenshots_dir, exist_ok=True) - self.window_size = window_size - self.touch_enabled = touch_enabled + self.window_size = test_class.browser_size + self.touch_enabled = test_class.touch_enabled self.sigxcpu_handler = None self._chrome_start() self._find_websocket() @@ -1247,7 +1286,41 @@ class ChromeBrowser: message, self._result ) elif 'test successful' in message: - self._result.set_result(True) + if self.test_class.allow_end_on_form: + self._result.set_result(True) + return + + qs = fchain( + self._websocket_send('DOM.getDocument', params={'depth': 0}, with_future=True), + lambda d: self._websocket_send("DOM.querySelector", params={ + 'nodeId': d['root']['nodeId'], + 'selector': '.o_form_editable', + }, with_future=True) + ) + @qs.add_done_callback + def _qs_result(fut): + # stupid dumbass chrome returns a nodeid of 0 when nothing + # is found + if fut.result()['nodeId']: + self.take_screenshot("unsaved_form_") + self._result.set_exception(ChromeBrowserException("""\ +Tour finished with an open form view in edition mode. + +Form views in edition mode are automatically saved when the page is closed, \ +which leads to stray network requests and inconsistencies.""")) + return + + try: + self._result.set_result(True) + except Exception: + # if the future was already failed, we're happy, + # otherwise swap for a new failed + if self._result.exception() is None: + self._result = Future() + self._result.set_exception(ChromeBrowserException( + "Tried to make the tour successful twice." + )) + def _handle_exception(self, exceptionDetails, timestamp): message = exceptionDetails['text'] @@ -1307,7 +1380,7 @@ class ChromeBrowser: decoded = base64.b64decode(base_png, validate=True) fname = '{}{:%Y%m%d_%H%M%S_%f}{}.png'.format( prefix, datetime.now(), - suffix or '_%s' % self.test_class) + suffix or '_%s' % self.test_class.__name__) full_path = os.path.join(self.screenshots_dir, fname) with open(full_path, 'wb') as f: f.write(decoded) @@ -1567,6 +1640,7 @@ class HttpCase(TransactionCase): browser = None browser_size = '1366x768' touch_enabled = False + allow_end_on_form = False _logger: logging.Logger = None @@ -1597,7 +1671,7 @@ class HttpCase(TransactionCase): def start_browser(cls): # start browser on demand if cls.browser is None: - cls.browser = ChromeBrowser(cls._logger, cls.browser_size, cls.touch_enabled, cls.__name__) + cls.browser = ChromeBrowser(cls) cls.addClassCleanup(cls.terminate_browser) @classmethod @@ -1747,7 +1821,7 @@ class HttpCase(TransactionCase): message = 'The test code "%s" failed' % code else: message = "Some js test failed" - self.fail('%s\n%s' % (message, error)) + self.fail('%s\n\n%s' % (message, error)) finally: # clear browser to make it stop sending requests, in case we call