Merge branch 'release/1.7.7'

.pylintrc

@@ -1,8 +1,9 @@
 [MESSAGES CONTROL]
-disable= R,attribute-defined-outside-init,bad-continuation,bad-option-value,bare-except,invalid-name,locally-disabled,missing-docstring,redefined-builtin,ungrouped-imports,wrong-import-order,wrong-import-position
+disable= R,attribute-defined-outside-init,bad-continuation,bad-option-value,bare-except,invalid-name,locally-disabled,missing-docstring,redefined-builtin,ungrouped-imports,wrong-import-order,wrong-import-position,unnecessary-pass
 
 [REPORTS]
 reports=no
+ignored-classes=ColorizedString
 
 [FORMAT]
 max-line-length=150

doc/changelog.rst

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+* :release:`1.7.7 <21-01-2019>`
+* :bug:`907` Fix parallel worker timeout issues
+* :bug:`-` Fix test_start triggering when exceptions are thrown in class-based tests (thanks @pierreluctg)
+* :bug:`-` Several Windows-specific fixes (thanks @pierreluctg)
+* :bug:`-` Fix swallowing of broken-pipe errors from console reporter - will not add errors
 * :release:`1.7.6 <16-12-2018>`
 * :bug:`-` Fix state saving of unstarted tests during interruptions
 * :bug:`-` Fix parallel execution on Windows systems (thanks @pierreluctg!)

slash/core/fixtures/fixture_store.py

@@ -68,17 +68,18 @@ def call_with_fixtures(self, test_func, namespace, trigger_test_start=False, tri
         else:
             kwargs = {}
 
-        if trigger_test_start:
-            for fixture in self.iter_active_fixtures():
-                fixture.call_test_start()
-
         try:
-            return test_func(**kwargs)
-        finally:
-            if trigger_test_end:
+            if trigger_test_start:
                 for fixture in self.iter_active_fixtures():
-                    with handling_exceptions(swallow=True):
-                        fixture.call_test_end()
+                    fixture.call_test_start()
+        finally:
+            try:
+                return test_func(**kwargs)  # pylint: disable=lost-exception
+            finally:
+                if trigger_test_end:
+                    for fixture in self.iter_active_fixtures():
+                        with handling_exceptions(swallow=True):
+                            fixture.call_test_end()
 
     def get_required_fixture_names(self, test_func):
         """Returns a list of fixture names needed by test_func.

slash/core/fixtures/namespace.py

@@ -24,7 +24,7 @@ def iter_fixtures(self):
         while self is not None:
             for fixture_id in itervalues(self._fixture_names):
                 yield self._store.get_fixture_by_id(fixture_id)
-            self = self._parent
+            self = self._parent  # pylint: disable=self-cls-assignment
 
     def __repr__(self):
         return 'Fixture NS#{}: {}'.format(self.get_level(), ', '.join(self._iter_fixture_names()) or '**None**')
@@ -33,13 +33,13 @@ def _iter_fixture_names(self):
         while self is not None:
             for k in self._fixture_names:
                 yield k
-            self = self._parent
+            self = self._parent  # pylint: disable=self-cls-assignment
 
     def get_fixture_by_name(self, name, default=NOTHING):
         while self is not None:
             fixture_id = self._fixture_names.get(name, NOTHING)
             if fixture_id is NOTHING:
-                self = self._parent
+                self = self._parent  # pylint: disable=self-cls-assignment
                 continue
             return self._store.get_fixture_by_id(fixture_id)
 

slash/core/local_config.py

@@ -48,9 +48,11 @@ def _traverse_upwards(self, path):
         if os.path.isfile(path):
             path = os.path.dirname(path)
 
+        upward_limit = os.path.splitdrive(os.path.normcase(os.path.abspath(os.path.sep)))[1]
+
         while True:
             yield path
-            if os.path.normcase(path) == os.path.normcase(os.path.abspath(os.path.sep)):
+            if os.path.splitdrive(os.path.normcase(path))[1] == upward_limit:
                 break
             new_path = os.path.dirname(path)
             assert new_path != path

slash/core/session.py

@@ -81,10 +81,10 @@ def activate(self):
             ctx.context.result = self.results.global_result
             self.results.global_result.mark_started()
             self._logging_context = self.logging.get_session_logging_context()
-            self._logging_context.__enter__()
+            self._logging_context.__enter__()  # https://github.com/PyCQA/pylint/issues/2056: pylint: disable=no-member
 
             self._warning_capture_context = self.warnings.capture_context()
-            self._warning_capture_context.__enter__()
+            self._warning_capture_context.__enter__()  # https://github.com/PyCQA/pylint/issues/2056: pylint: disable=no-member
         self._active = True
 
     def deactivate(self):

slash/frontend/slash_list.py

@@ -108,7 +108,7 @@ def _report_tests(args, runnables, printer):
 
 
 def _convert_address_to_relpath(address):
-    filename, remainder = address.split(':', 1)
+    filename, remainder = address.rsplit(':', 1)
     if os.path.isabs(filename):
         filename = os.path.relpath(filename)
     return '{}:{}'.format(filename, remainder)

slash/parallel/parallel_manager.py

@@ -15,7 +15,6 @@
 from .server import Server, ServerStates, KeepaliveServer
 from ..utils.tmux_utils import create_new_window, create_new_pane
 from .worker_configuration import WorkerConfiguration
-from .._compat import iteritems
 from .. import hooks
 
 _logger = logbook.Logger(__name__)
@@ -128,8 +127,12 @@ def wait_all_workers_to_connect(self):
             time.sleep(TIME_BETWEEN_CHECKS)
 
     def check_worker_timed_out(self):
-        for worker_id, last_connection_time in iteritems(self.keepalive_server.get_workers_last_connection_time()):
-            if time.time() - last_connection_time > config.root.parallel.communication_timeout_secs:
+        workers_last_connection_time = self.keepalive_server.get_workers_last_connection_time()
+        for worker_id in self.server.connected_clients:
+            worker_last_connection_time = workers_last_connection_time.get(worker_id, None)
+            if worker_last_connection_time is None: #worker keepalive thread didn't started yet
+                continue
+            if time.time() - worker_last_connection_time > config.root.parallel.communication_timeout_secs:
                 _logger.error("Worker {} is down, terminating session", worker_id, extra={'capture': False})
                 self.report_worker_error_logs()
                 if not config.root.tmux.enabled:

slash/parallel/server.py

@@ -1,6 +1,7 @@
 import copy
 import logbook
 import time
+import threading
 from enum import Enum
 from six.moves import queue
 from six.moves import xmlrpc_server
@@ -33,9 +34,11 @@ def __init__(self):
         self.last_request_time = time.time()
         self.state = ServerStates.NOT_INITIALIZED
         self.port = None
+        self._lock = threading.Lock()
 
     def keep_alive(self, client_id):
-        self.clients_last_communication_time[client_id] = self.last_request_time = time.time()
+        with self._lock:
+            self.clients_last_communication_time[client_id] = self.last_request_time = time.time()
         _logger.debug("Client_id {} sent keep_alive", client_id)
 
     def stop_serve(self):
@@ -55,7 +58,8 @@ def serve(self):
             server.server_close()
 
     def get_workers_last_connection_time(self):
-        return copy.deepcopy(self.clients_last_communication_time)
+        with self._lock:
+            return copy.deepcopy(self.clients_last_communication_time)
 
 
 class Server(object):

slash/reporting/console_reporter.py

@@ -16,7 +16,6 @@
 from ..utils.iteration import iteration
 from ..utils.python import wraps
 from .reporter_interface import ReporterInterface
-from ..exception_handling import handling_exceptions
 
 # traceback levels
 NO_TRACEBACK, SINGLE_FRAME, ALL_FRAMES, ALL_FRAMES_WITH_CONTEXT, ALL_FRAMES_WITH_CONTEXT_AND_VARS = range(
@@ -41,8 +40,10 @@ def new_func(self, *args, **kwargs):
 def swallowing_terminal_exceptions(func):
     @functools.wraps(func)
     def inner(*args, **kwargs):
-        with handling_exceptions(swallow_types=(IOError, OSError), swallow=True):
+        try:
             return func(*args, **kwargs)
+        except (IOError, OSError):
+            pass
     return inner
 
 

slash/runner.py

@@ -224,6 +224,7 @@ def _get_test_context(test, logging=True):
         prev_result = context.result
         context.result = result
         try:
+            # pylint: disable=superfluous-parens
             with (context.session.logging.get_test_logging_context(result) if logging else ExitStack()):
                 _logger.debug("Started test #{0.__slash__.test_index1}: {0}", test)
                 yield result, prev_result

tests/test_cli_utils.py

@@ -32,7 +32,7 @@ def test_printer_with_forced_colored():
     for string, style in [('A', _style_1), ('B', _style_2), ('C', None)]:
         colored_string = _colorized(string, style)
         printer(colored_string)
-        expected_lines.append(colored_string.colorize() if style else str(colored_string))
+        expected_lines.append(colored_string.colorize() if style else str(colored_string))  # pylint: disable=no-member
     assert report_stream.getvalue().splitlines() == expected_lines
 
 

tests/test_error_object_stack_trace.py

@@ -56,7 +56,7 @@ def h1():
 
     with slash.Session() as s:
         f1()
-    [err] = s.results.global_result.get_errors()
+    [err] = s.results.global_result.get_errors()  # pylint: disable=unbalanced-tuple-unpacking
     return err
 
 

tests/test_ext_hook.py

@@ -22,8 +22,8 @@ def test_ext_hook_import(self):
         self.assertEqual(module.value, self.expected_value)
 
     def test_slash_ext(self):
-        from slash import ext   # pylint: disable=unused-variable
+        from slash import ext   # pylint: disable=unused-variable,unused-import
 
     def test_ext_hook_import_nonexistent(self):
         with self.assertRaises(ImportError):
-            from slash.ext import nonexistent  # pylint: disable=unused-variable, no-name-in-module
+            from slash.ext import nonexistent  # pylint: disable=unused-variable,no-name-in-module,unused-import

tests/test_fixture_mechanism.py

@@ -477,7 +477,7 @@ def some_fixture():
 
     store.resolve()
 
-    assert store.get_fixture_by_name('custom').fixture_func == some_fixture
+    assert store.get_fixture_by_name('custom').fixture_func is some_fixture
     with pytest.raises(UnknownFixtures):
         assert store.get_fixture_by_name('some_fixture')
 

tests/test_fixture_start_end_test.py

@@ -30,7 +30,7 @@ def test_fixture_start_end_test(suite, suite_test, scope, error_adder):
     assert events[end_event].timestamp > events[test_event].timestamp
 
 
-def test_fixture_end_test_raises_excepiton(suite_builder):
+def test_fixture_end_test_raises_exception(suite_builder):
     @suite_builder.first_file.add_code
     def __code__():
         # pylint: disable=unused-variable
@@ -51,3 +51,80 @@ def test_something(fixture):
     suite_builder.build().run().assert_all(2).exception(ZeroDivisionError).with_data(
         [{"value": 1}, {"value": 2}]
     )
+
+
+def test_fixture_start_test_raises_exception(suite_builder):
+    @suite_builder.first_file.add_code
+    def __code__():
+        # pylint: disable=unused-variable
+        import slash  # pylint: disable=redefined-outer-name, reimported
+
+        @slash.fixture(scope='module')
+        @slash.parametrize("param", [1, 2])
+        def fixture(this, param):
+            @this.test_start
+            def test_start(*_, **__):
+                1 / 0  # pylint: disable=pointless-statement
+
+            return param
+
+        @slash.fixture(scope='module')
+        def fixture_failing(this):
+            @this.test_start
+            def test_start(*_, **__):
+                raise AssertionError()
+
+            return None
+
+        class SomeTest(slash.Test):
+            @slash.parametrize("param", [10, 20, 30])
+            def test_something(self, param, fixture, fixture_failing):  # pylint: disable=unused-argument
+                slash.context.result.data["value"] = param + fixture
+
+    suite_builder.build().run().assert_all(6).exception(ZeroDivisionError).with_data(
+        [
+            {"value": 11}, {"value": 12},
+            {"value": 21}, {"value": 22},
+            {"value": 31}, {"value": 32}
+        ]
+    )
+
+
+def test_fixture_start_test_raises_exception_w_before(suite_builder):
+    @suite_builder.first_file.add_code
+    def __code__():
+        # pylint: disable=unused-variable
+        import slash  # pylint: disable=redefined-outer-name, reimported
+
+        @slash.fixture(scope='module')
+        @slash.parametrize("param", [1, 2])
+        def fixture(this, param):
+            @this.test_start
+            def test_start(*_, **__):
+                1 / 0  # pylint: disable=pointless-statement
+
+            return param
+
+        @slash.fixture(scope='module')
+        def fixture_failing(this):
+            @this.test_start
+            def test_start(*_, **__):
+                raise AssertionError()
+
+            return None
+
+        class SomeTest(slash.Test):
+            def before(self, fixture, fixture_failing):  # pylint: disable=unused-argument,arguments-differ
+                self.data = fixture
+
+            @slash.parametrize("param", [10, 20, 30])
+            def test_something(self, param):
+                slash.context.result.data["value"] = param + self.data
+
+    suite_builder.build().run().assert_all(6).exception(ZeroDivisionError).with_data(
+        [
+            {"value": 11}, {"value": 12},
+            {"value": 21}, {"value": 22},
+            {"value": 31}, {"value": 32}
+        ]
+    )

tests/test_hook_calling.py

@@ -213,7 +213,7 @@ def hook():
             with s.get_started_context():
                 pass
 
-    [err] = s.results.global_result.get_errors()
+    [err] = s.results.global_result.get_errors()  # pylint: disable=unbalanced-tuple-unpacking
     assert 'CustomException' in str(err)
     assert not checkpoint.called
 

tests/test_parallel.py

@@ -67,6 +67,15 @@ def test_keepalive_works(parallel_suite, config_override):
     assert len(summary.session.parallel_manager.server.worker_session_ids) == workers_num
     assert summary.session.results.is_success()
 
+def test_disconnected_worker_not_considered_timed_out(parallel_suite, config_override):
+    config_override("parallel.communication_timeout_secs", 2)
+    parallel_suite[0].append_line("import time")
+    parallel_suite[0].append_line("time.sleep(6)")
+    workers_num = 2
+    summary = parallel_suite.run(num_workers=workers_num)
+    assert len(summary.session.parallel_manager.server.worker_session_ids) == workers_num
+    assert summary.session.results.is_success()
+
 def test_server_fails(parallel_suite):
 
     @slash.hooks.worker_connected.register  # pylint: disable=no-member, unused-argument

tests/test_parametrization_labels.py

@@ -118,3 +118,23 @@ def test_1(param):
         {'value': 'value1'},
         {'value': 'value2'},
     ])
+
+
+def test_fixture_param_labels(suite_builder):
+    # pylint: disable=no-member, protected-access, undefined-variable,unused-variable, reimported, redefined-outer-name
+    @suite_builder.first_file.add_code
+    def __code__():
+        import slash
+
+        @slash.fixture
+        @slash.parametrize('value', [1 // slash.param('first'), 2 // slash.param('second')])
+        def fixture(value):
+            return value * 2
+
+        def test_1(fixture):
+            slash.context.result.data['value'] = fixture
+
+    suite_builder.build().run().assert_success(2).with_data([
+        {'value': 2},
+        {'value': 4},
+    ])

tests/test_warning_ignored.py

@@ -24,7 +24,7 @@ class CustomWarning(UserWarning):
     (_warn('hello'), _catch(message=re.compile('^hello$')), True),
     (_warn('message', category=CustomWarning), _catch(category=CustomWarning), True),
     (_warn('message'), _catch(filename=__file__), False),
-    (_warn('message'), _catch(filename=re.compile('^{}$'.format(__file__))), False),
+    (_warn('message'), _catch(filename=re.compile('^{}$'.format(re.escape(__file__)))), False),
 ])
 def test_ignore_warnings(emitter, catch, can_test_negative):
     catch()
@@ -45,13 +45,13 @@ def test_ignore_warnings(emitter, catch, can_test_negative):
 @pytest.mark.parametrize('catch,should_ignore', [
     (_catch(message=_WARN_MESSAGE, filename=__file__), True),
     (_catch(message=re.compile('^Hello.*$'), filename=__file__), True),
-    (_catch(message=_WARN_MESSAGE, filename=re.compile('^{}'.format(os.path.dirname(__file__)))), True),
-    (_catch(message=re.compile('^Hello.*$'), filename=re.compile('^{}'.format(os.path.dirname(__file__)))), True),
+    (_catch(message=_WARN_MESSAGE, filename=re.compile('^{}'.format(re.escape(os.path.dirname(__file__))))), True),
+    (_catch(message=re.compile('^Hello.*$'), filename=re.compile('^{}'.format(re.escape(os.path.dirname(__file__))))), True),
     # Negative  (OR relation instead of AND)
     (_catch(message=_WARN_MESSAGE, category=DeprecationWarning), False),
     (_catch(message=re.compile('^Hello.*$'), category=DeprecationWarning), False),
     (_catch(filename=__file__, category=DeprecationWarning), False),
-    (_catch(filename=re.compile('^{}$'.format(__file__)), category=DeprecationWarning), False),
+    (_catch(filename=re.compile('^{}$'.format(re.escape(__file__))), category=DeprecationWarning), False),
 ])
 def test_ignore_warnings_with_multiple_criteria(catch, should_ignore):
     catch()