diff --git a/.coveragerc b/.coveragerc index 3e94abf34..81c8abff8 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,5 +1,6 @@ [run] branch = True +parallel = True source = cloudpickle [report] diff --git a/.travis.yml b/.travis.yml index 4bb9f6174..cc3c3bd8a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,9 +23,11 @@ before_script: - flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide - flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - python ci/install_coverage_subprocess_pth.py script: - if [[ $TRAVIS_PYTHON_VERSION != 'pypy'* ]]; then source activate testenv; fi - - PYTHONPATH='.:tests' py.test -r s --cov-config .coveragerc --cov=cloudpickle + - COVERAGE_PROCESS_START="$TRAVIS_BUILD_DIR/.coveragerc" PYTHONPATH='.:tests' py.test -r s after_success: - if [[ $TRAVIS_PYTHON_VERSION != 'pypy'* ]]; then source activate testenv; fi + - coverage combine --append - codecov diff --git a/ci/install_coverage_subprocess_pth.py b/ci/install_coverage_subprocess_pth.py new file mode 100644 index 000000000..ec93abcaa --- /dev/null +++ b/ci/install_coverage_subprocess_pth.py @@ -0,0 +1,16 @@ +# Make it possible to enable test coverage reporting for Python +# code run in children processes. +# http://coverage.readthedocs.io/en/latest/subprocess.html + +import os.path as op +from distutils.sysconfig import get_python_lib + +FILE_CONTENT = u"""\ +import coverage; coverage.process_startup() +""" + +filename = op.join(get_python_lib(), 'coverage_subprocess.pth') +with open(filename, 'wb') as f: + f.write(FILE_CONTENT.encode('ascii')) + +print('Installed subprocess coverage support: %s' % filename) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index 33a431d47..5ec7d0267 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -431,6 +431,7 @@ def _save_subimports(self, code, top_level_dependencies): Ensure de-pickler imports any package child-modules that are needed by the function """ + # check if any known dependency is an imported package for x in top_level_dependencies: if isinstance(x, types.ModuleType) and hasattr(x, '__package__') and x.__package__: @@ -623,7 +624,15 @@ def extract_func_data(self, func): # save the dict dct = func.__dict__ - base_globals = self.globals_ref.get(id(func.__globals__), {}) + base_globals = self.globals_ref.get(id(func.__globals__), None) + if base_globals is None: + # For functions defined in __main__, use vars(__main__) for + # base_global. This is necessary to share the global variables + # across multiple functions in this module. + if func.__module__ == "__main__": + base_globals = "__main__" + else: + base_globals = {} self.globals_ref[id(func.__globals__)] = base_globals return (code, f_globals, defaults, closure, dct, base_globals) @@ -1028,7 +1037,11 @@ def _fill_function(*args): else: raise ValueError('Unexpected _fill_value arguments: %r' % (args,)) - func.__globals__.update(state['globals']) + # Only set global variables that do not exist. + for k, v in state['globals'].items(): + if k not in func.__globals__: + func.__globals__[k] = v + func.__defaults__ = state['defaults'] func.__dict__ = state['dict'] if 'annotations' in state: @@ -1067,6 +1080,8 @@ def _make_skel_func(code, cell_count, base_globals=None): """ if base_globals is None: base_globals = {} + elif isinstance(base_globals, str): + base_globals = vars(sys.modules[base_globals]) base_globals['__builtins__'] = __builtins__ closure = ( diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 8826bac19..a591997a4 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -841,6 +841,45 @@ def f4(x): """.format(protocol=self.protocol) assert_run_python_script(textwrap.dedent(code)) + def test_interactively_defined_global_variable(self): + # Check that callables defined in the __main__ module of a Python + # script (or jupyter kernel) correctly retrieve global variables. + code_template = """\ + from testutils import subprocess_pickle_echo + from cloudpickle import dumps, loads + + def local_clone(obj, protocol=None): + return loads(dumps(obj, protocol=protocol)) + + VARIABLE = "default_value" + + def f0(): + global VARIABLE + VARIABLE = "changed_by_f0" + + def f1(): + return VARIABLE + + cloned_f0 = {clone_func}(f0, protocol={protocol}) + cloned_f1 = {clone_func}(f1, protocol={protocol}) + pickled_f1 = dumps(f1, protocol={protocol}) + + # Change the value of the global variable + cloned_f0() + + # Ensure that the global variable is the same for another function + result_f1 = cloned_f1() + assert result_f1 == "changed_by_f0", result_f1 + + # Ensure that unpickling the global variable does not change its value + result_pickled_f1 = loads(pickled_f1)() + assert result_pickled_f1 == "changed_by_f0", result_pickled_f1 + """ + for clone_func in ['local_clone', 'subprocess_pickle_echo']: + code = code_template.format(protocol=self.protocol, + clone_func=clone_func) + assert_run_python_script(textwrap.dedent(code)) + @pytest.mark.skipif(sys.version_info >= (3, 0), reason="hardcoded pickle bytes for 2.7") def test_function_pickle_compat_0_4_0(self): diff --git a/tests/testutils.py b/tests/testutils.py index a8187baf3..2a14e86c7 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -93,6 +93,10 @@ def assert_run_python_script(source_code, timeout=5): 'stderr': STDOUT, 'env': {'PYTHONPATH': pythonpath}, } + # If coverage is running, pass the config file to the subprocess + coverage_rc = os.environ.get("COVERAGE_PROCESS_START") + if coverage_rc: + kwargs['env']['COVERAGE_PROCESS_START'] = coverage_rc if timeout_supported: kwargs['timeout'] = timeout try: