Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions tests/cloudpickle_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ def method(self, x):
mod1, mod2 = pickle_depickle([mod, mod])
self.assertEqual(id(mod1), id(mod2))

def test_dynamic_modules_cache(self):
def test_dynamic_modules_globals(self):
# _dynamic_modules_globals is a WeakValueDictionary, so if this dynamic
# module has no other reference than in this # dict (whether on the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# leftover

# child process or the parent process), this module will be garbage
Expand All @@ -452,23 +452,37 @@ def test_dynamic_modules_cache(self):
mod = imp.new_module('mod')
code = '''
x = 1
def f():
def func():
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need the function to reference and mutate the global variable as we did in #188 and #198. Otherwise we cannot make assertions on the correct handling of the global variables.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe test_function_from_dynamic_module_with_globals_modifications is already testing this behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is. I can merge the two test, but unsure if it is better.

'''
exec(textwrap.dedent(code), mod.__dict__)

pickled_module_path = 'mod.pkl'
print(mod.func.__module__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spurious print


with open(pickled_module_path, 'wb') as f:
cloudpickle.dump(mod, f)
cloudpickle.dump(mod.func, f)

# _dynamic_modules_globals will have mod appended to its values
# in the child process only
child_process_script = '''
import pickle
import cloudpickle
from cloudpickle.cloudpickle import _dynamic_modules_globals
import gc
with open("{pickled_module_path}", 'rb') as f:
_ = pickle.load(f)
func = pickle.load(f)

# A dictionnary storing the globals of the newly unpickled function
# should have been created
assert list(_dynamic_modules_globals.keys())==['mod']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: whitespaces around ==.


del func
gc.collect()

# There is no reference to the globals of func since func has been
# deleted and _dynamic_modules_globals is a WeakValueDictionary,
# so _dynamic_modules_globals should now be empty
assert list(_dynamic_modules_globals.keys())==[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem

'''
child_process_script = child_process_script.format(
pickled_module_path=pickled_module_path)
Expand All @@ -477,12 +491,6 @@ def f():

# We remove the other references to the object and trigger a
# gc event
del mod
gc.collect()

# At this point, there is no other reference to this module, so
# _dynamic_modules_globals should be empty
assert list(_dynamic_modules_globals.keys()) == []

def test_load_dynamic_module_in_grandchild_process(self):
# Make sure that when loaded, a dynamic module preserves its dynamic
Expand Down