pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] core: skip marking completed tours as failed, restore had_failure

Browse Source 
Succeeds #140464

Misunderstood `had_failure` and should not have reused it, its goal is
to avoid eagerly aborting some JS tests -- specifically the unit test
suites -- while still logging errors normally (useful when watching
interactively, or for the runbot's own reporting).

So the *checks* added on `had_failure` should in fact be checks on
`_result.exception()`, and as it turns out on `_result.done()`: if a
tour is already marked as successful we can't fail it either.

So we should not, we should log an error (to notify the caller /
runbot) and then bail. While #140464 did improve some things, we could
still lose legit errors and get pages of unhelpful `InvalidStateError`
if a tour would succeed *then* failures would occur, as the guard only
checked that the tour had already failed.

closes odoo/odoo#141815

X-original-commit: 87fcf66203dcbd281470285d648365d33d0e2fca
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
This commit is contained in:
Xavier Morel
2023-11-13 14:38:40 +00:00
parent 7b95f5c8a7
commit bc81230311
1 changed files with 20 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
odoo/tests/common.py
+20 -20
View File
@@ -917,6 +917,7 @@ class ChromeBrowser:
self._request_id = itertools.count()
self._result = Future()
self.error_checker = None
self.had_failure = False
# maps request_id to Futures
self._responses = {}
# maps frame ids to callbacks
@@ -942,12 +943,6 @@ class ChromeBrowser:
def screencasts_frames_dir(self):
return os.path.join(self.screencasts_dir, 'frames')
@property
def had_failure(self):
with contextlib.suppress(concurrent.futures.TimeoutError, CancelledError):
return self._result.exception(timeout=0) is not None
return False
def signal_handler(self, sig, frame):
if sig == signal.SIGXCPU:
_logger.info('CPU time limit reached, stopping Chrome and shutting down')
@@ -1243,13 +1238,17 @@ class ChromeBrowser:
message += '\n' + stack
log_type = type
self._logger.getChild('browser').log(
_logger = self._logger.getChild('browser')
_logger.log(
self._TO_LEVEL.get(log_type, logging.INFO),
"%s", message # might still have %<x> characters
"%s%s",
"Error received after termination: " if self._result.done() else "",
message # might still have %<x> characters
)
if log_type == 'error':
if self.had_failure:
self.had_failure = True
if self._result.done():
return
if not self.error_checker or self.error_checker(message):
self.take_screenshot()
@@ -1283,23 +1282,23 @@ class ChromeBrowser:
if node_id:
self.take_screenshot("unsaved_form_")
self._result.set_exception(ChromeBrowserException("""\
msg = """\
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."""))
which leads to stray network requests and inconsistencies."""
if self._result.done():
_logger.error("%s", msg)
else:
self._result.set_exception(ChromeBrowserException(msg))
return
try:
if not self._result.done():
self._result.set_result(True)
except Exception:
elif self._result.exception() is None:
# 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."
))
_logger.error("Tried to make the tour successful twice.")
def _handle_exception(self, exceptionDetails, timestamp):
@@ -1312,8 +1311,9 @@ which leads to stray network requests and inconsistencies."""))
if stack:
message += '\n' + stack
if self.had_failure:
self._logger.getChild('browser').error("%s", message)
if self._result.done():
self._logger.getChild('browser').error(
"Exception received after termination: %s", message)
return
self.take_screenshot()