From 0b116729f7245edc08f7e6645d7ded2a8bd45bca Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 14 Jan 2016 20:32:17 +0100 Subject: [PATCH] bdd: Fix skipping tests from JS. With the previous solution, we'd call pytest.skip on teardown only, which means the rest of the test (e.g. because of a wait_for) could still fail. --- tests/integration/quteprocess.py | 45 ++++++++++++++------------- tests/integration/test_quteprocess.py | 27 +++++++++------- tests/integration/testprocess.py | 16 ++++++++++ 3 files changed, 55 insertions(+), 33 deletions(-) diff --git a/tests/integration/quteprocess.py b/tests/integration/quteprocess.py index e1e164b9d..74d3487b0 100644 --- a/tests/integration/quteprocess.py +++ b/tests/integration/quteprocess.py @@ -238,32 +238,35 @@ class QuteProc(testprocess.Process): value=msg.message)) return msg.loglevel > logging.INFO or is_js_error - def _is_js_skip_logline(self, msg): - """Check if a JS snippet wanted to skip a test.""" - return (msg.category == 'js' and - msg.function == 'javaScriptConsoleMessage' and - testutils.pattern_match(pattern='[*] [SKIP] *', - value=msg.message)) + def _maybe_skip(self): + """Skip the test if [SKIP] lines were logged.""" + skip_texts = [] + + for msg in self._data: + if (msg.category == 'js' and + msg.function == 'javaScriptConsoleMessage' and + testutils.pattern_match(pattern='[*] [SKIP] *', + value=msg.message)): + skip_texts.append(msg.message.partition(' [SKIP] ')[2]) + + if skip_texts: + pytest.skip(', '.join(skip_texts)) def after_test(self): bad_msgs = [msg for msg in self._data if self._is_error_logline(msg) and not msg.expected] - skip_msgs = [msg for msg in self._data - if self._is_js_skip_logline(msg)] - super().after_test() - - if bad_msgs: - text = 'Logged unexpected errors:\n\n' + '\n'.join( - str(e) for e in bad_msgs) - # We'd like to use pytrace=False here but don't as a WORKAROUND - # for https://github.com/pytest-dev/pytest/issues/1316 - pytest.fail(text) - elif skip_msgs: - texts = [] - for msg in skip_msgs: - texts.append(msg.message.partition(' [SKIP] ')[2]) - pytest.skip(', '.join(texts)) + try: + if bad_msgs: + text = 'Logged unexpected errors:\n\n' + '\n'.join( + str(e) for e in bad_msgs) + # We'd like to use pytrace=False here but don't as a WORKAROUND + # for https://github.com/pytest-dev/pytest/issues/1316 + pytest.fail(text) + else: + self._maybe_skip() + finally: + super().after_test() def send_cmd(self, command, count=None): """Send a command to the running qutebrowser instance.""" diff --git a/tests/integration/test_quteprocess.py b/tests/integration/test_quteprocess.py index 2b9a14b8d..09645e7fa 100644 --- a/tests/integration/test_quteprocess.py +++ b/tests/integration/test_quteprocess.py @@ -43,21 +43,24 @@ def test_quteproc_error_message(qtbot, quteproc, cmd): quteproc.after_test() -@pytest.mark.parametrize('texts, expected', [ - (["[SKIP] test"], "test"), - (["[SKIP] foo", "[SKIP] bar"], "foo, bar"), -]) -def test_quteproc_skip_via_js(qtbot, quteproc, texts, expected): - for text in texts: - quteproc.send_cmd(':jseval console.log("{}");'.format(text)) - quteproc.wait_for_js(text) - - # Usually we wouldn't call this from inside a test, but here we force the - # error to occur during the test rather than at teardown time. +def test_quteproc_skip_via_js(qtbot, quteproc): with pytest.raises(pytest.skip.Exception) as excinfo: + quteproc.send_cmd(':jseval console.log("[SKIP] test");') + quteproc.wait_for_js('[SKIP] test') + + # Usually we wouldn't call this from inside a test, but here we force + # the error to occur during the test rather than at teardown time. quteproc.after_test() - assert str(excinfo.value) == expected + assert str(excinfo.value) == 'test' + + +def test_quteproc_skip_and_wait_for(qtbot, quteproc): + """This test will skip *again* during teardown, but we don't care.""" + with pytest.raises(pytest.skip.Exception): + quteproc.send_cmd(':jseval console.log("[SKIP] foo");') + quteproc.wait_for_js("[SKIP] foo") + quteproc.wait_for(message='This will not match') def test_qt_log_ignore(qtbot, quteproc): diff --git a/tests/integration/testprocess.py b/tests/integration/testprocess.py index 9668f1689..67896302d 100644 --- a/tests/integration/testprocess.py +++ b/tests/integration/testprocess.py @@ -324,6 +324,19 @@ class Process(QObject): return line return None + def _maybe_skip(self): + """Can be overridden by subclasses to skip on certain log lines. + + We can't run pytest.skip directly while parsing the log, as that would + lead to a pytest.skip.Exception error in a virtual Qt method, which + means pytest-qt fails the test. + + Instead, we check for skip messages periodically in + QuteProc._maybe_skip, and call _maybe_skip after every parsed message + in wait_for (where it's most likely that new messages arrive). + """ + pass + def wait_for(self, timeout=None, *, override_waited_for=False, do_skip=False, **kwargs): """Wait until a given value is found in the data. @@ -342,6 +355,7 @@ class Process(QObject): The matched line. """ __tracebackhide__ = True + if timeout is None: if do_skip: timeout = 2000 @@ -365,6 +379,8 @@ class Process(QObject): elapsed_timer.start() while True: + # Skip if there are pending messages causing a skip + self._maybe_skip() got_signal = spy.wait(timeout) if not got_signal or elapsed_timer.hasExpired(timeout): msg = "Timed out after {}ms waiting for {!r}.".format(