Sentinels lose their identity when they are pickled, meaning they are no longer equal to sentinel with the same name.


>>> from mock import sentinel
>>> import pickle
>>> sentinel.foo == sentinel.foo
True
>>> pickle.loads(pickle.dumps(sentinel.foo)) == sentinel.foo
False


I found this bug using Python 2.7.3 and python-mock 1.0.1. Since mock 1.0 is part of Python 3.3 and 3.4 it affects these versions (tested in 3.3.3).

This behavior is tricky to find out if encountered in tests and sentinels can not be used in tests of code which uses pickle and possibly other serializations.

Since the point of a sentinel is *object identity*, there is no way that you can get the "same" sentinel back after a pickle.  This is fundamental to the nature of pickle.

I don't question pickle, but I'd expect sentinels to keep their equality when they are pickled or copied.

Sentinels are not equal unless they are the *same object*, just as for any python object that does not have a defined __eq__.  And as I said, this is part of the point of sentinels.

Maybe making sentinels unpickleable would be a better solution. On the other hand I'd love to be able to [properly] customise object creation when unpickling. It would solve various other problems I've had from time to time. But that's well outside the scope of this bug.

In case anyone is wondering, I'm leaving this open in case Michael thinks it is a good idea to add some sort of documentation note about this.  I'm not sure it is, because this is fundamental to the way Python works.  On the other hand, I can't think of any specific manual or tutorial section we could direct someone to to get an explanation of what is going on.

This one comes close: http://docs.python.org/2/faq/programming.html#how-can-my-code-discover-the-name-of-an-object but doesn't tell the whole story since it doesn't cover pickle.

Our posts crossed.

Making them unpickleable sounds like it might be sensible.  Since the sentinel *can't* survive pickling/unpickling and still perform its documented function, it seems better to raise an error if someone tries.

From looking at the sentinel code it appears a sentinel's identity is controlled by its name alone, i. e.

sentinel.foo is sentinel.foo

If that's the desired behaviour it is well possible to make that indentity survive pickling. I have attached a demo script using the simplest approach -- registering the class with copyreg.
If you like the general idea I'm willing to look up the alternative (a __getstate__() or __reduce__() method), too ;)

Personally I think that's a bad idea.  Anything that implies that object identity can survive pickle/unpickle is IMO bad, since pickle is a serialization protocol and in real life (as opposed to tests) the process that does the unpickle is almost always not the process that does the pickle.

I have no experience with unittest.mock, so I'm in no position to contradict... Vlastimil, could you give your use case?

I encountered this problem when I tested code with cache, something like

    def test_foo(self):
        cache.set('a_key', sentinel.status) # Performs pickle
        self.assertEqual(
            cache.get('a_key'), # Performs unpickle
            sentinel.status)

I spent some time searching for reason why the test failed. Since both sentinels represent themselves as 'sentinel.status', I was quite surprised by assertion failure. Only after I looked at the code I realized where is the problem. Two sentinels are equal if and only if they are identical.

On my way home I realized this probably is and needs to be a feature of sentinels. Consider testing this function:

    def foo(value, do_copy):
        if do_copy:
            return bar(copy(value))
        else:
            return bar(value)

You would like to know whether `bar` was called with value or its copy. Sentinel is in a test for such function a great option to make sure `value` is not copied in process. This is especially usefull is you write tests for wrapper-like function which should only pass arguments (or a subset) to other interface.


After reading through comments and thinking about the problem I have following opinions:
 * Sentinels should not actually survive pickle/unpickle. 
 * Prevent sentinels from being pickled is a good idea, it is a valid way to tell developer he does something he shouldn't. This might also apply to copying, which would result in the same problem.
 * It should be noted in documentation that sentinels are equal only if identical. Since they represent themselves by their name it is hard to guess the name is only for the representation. Also I find the documentation snippet

>>> assert result is sentinel.some_object

somewhat misleading as you don't usually use 'is' for comparison and it provides no information about equality.

Yes, exactly, you don't use is for equality comparison.  It tests object identity, which is why it makes sense to use it with a sentinel.

The 'name' of the sentinel is purely a way to store and retrieve particular sentinel objects.  The sentinel itself does not have a name.

Good point about copy...the copy protocol and the pickle protocol are closely related, so preventing a sentinel from being pickled may prevent it from being copied as well.  It is an interesting question how surprising people will find it for a sentinel to throw an error if an attempt is made to copy it, but if the test is using a sentinel, presumably it expects the sentinel to survive unchanged, so an error on copy is probably a reasonable result.  You could imagine someone wanting to use a sentinel to test that copy happens by making sure they get back a different object, though, so it is not entirely clear cut.  However, I expect that to be *much* less common than wanting to use it to prove that an object is preserved (not copied).

David, first part of your comment should be added to docs. Purpose of sentinels isn't much intuitive and in my opinion documentation doesn't quite describe it.

An exception in case of pickle or copy would be much easier to understand apart from unexpected inequality. In my experience negative test in general are uncommon, so I expect usage of sentinels to ensure different objects to be rare.

I'm not sure to what extent mock documentation should explain Python semantics. The docs *do* make clear that sentinel is useful for where you want to test (compare) objects by *identity*. Problems with copying and pickling them come from the fact that those operations don't preserve *identity*, which is a basic facet of *Python* and not down to mock/sentinel.

"sentinel provides a convenient way of creating and testing the identity of objects like this."

Proposed patch makes mock sentinels supporting pickling and copying. This allows passing them in multiprocessing.

I'm not sure whether this is a bug fix or a new feature.

The patch works only with 3.5+. Older versions need more complex solution.

Serhiy: The above discussion seemed to converge on the perspective that object identity should not survive pickling and that the point of a sentinel is object identity.  While your proposed patch may mechanically work, I believe it is in conflict with the outcome of the thoughtful discussion above.

Sorry, I had wrote this patch for issue29229 and didn't read the above discussion.

Copying and pickling preserve identity of some objects: functions, classes, enums, builtin singletons (None, False, True, ...). It was not obvious to me that this shouldn't work with mock sentinels.

Since the issue was dug up, I've created a patch for documentation.

I still think, that it is worth stating the equality is not preserved for sentinels when copied as the message produced by tests is rather confusing:

    class SentinelTestCase(TestCase):
        def test_sentinel(self):
            self.assertEqual(sentinel.foo, copy(sentinel.foo))
            # AssertionError: sentinel.foo != sentinel.foo

So my thinking has evolved slightly on it. If it's *possible* for sentinels to be copied and pickled and preserve identity then I'm happy for it. I think the right semantics for copying a sentinel is that you get the original object back. If you pickle a sentinel and then unpickle back *into the same process* you should get the same object back. 

David worried that this confuses the semantics of pickling/copying because it isn't generally the case - so I'm open to further discussion on it, but if Serhiy has fixed the "problem" here I'm happy for it to go in.

I just don't know well the purpose of sentinels, and don't . I see fourth possible options:

1. Raise an exception when copy or pickle a sentinel. This is an option when we want to be sure that copying and pickling are not involved and we get the same object as passed.

2. Create an unique sentinel with unique unambiguous name when copy or unpickle a sentinel. This is a behavior of commonly used sentinels (object(), ['sentinel'], etc), and current behavior of mock.sentinel (except that the name is left the same and this is confusing).

3. Preserve identity when copy or pickle/unpickle a sentinel. This is an option when test a function that uses multiprocessing or persistent cache and should restore an equivalent object. This is a behavior of enums, classes, functions or other global named objects.

4. Preserve equality but not identity when copy or pickle/unpickle a sentinel. This is similar to the third option.

All options look reasonable to me. I don't know what case is more common, what behavior is more expectable.

I think option 3 is the correct semantic behaviour for sentinels, and if there are already examples of this in the standard library then it *doesn't* violate expected behaviour of pickling and copying (sentinel and singleton objects can be permitted to retain this property through copying and pickling if it is a document facet of their behaviour and a natural result of their use cases).

Sentinels exist *purely* to have unique, *identifiable* objects. So retaining identity is their "expected behaviour" as identity is their defining feature. 

So Serhiy, if you're happy that you've implemented this correctly - with tests and documentation updates (I see a separate doc patch) - go ahead and commit.

sentinel-doc.patch documents the contrary behaviour -- not preserving an identity and equality.

I haven't included doc changes because I don't know how to treat this change. As a bug fix, as a new feature, or as a new feature backported to maintained releases? If this is a new feature, it needs an entry in Whant's New and a versionchanged directive in the module documentation. If this is a bug fix, it likely doesn't need any special documenting.

It's a new feature.

Updated patch contains doc changes.

LGTM Serhiy - thank you for your work. Appreciated.

New changeset 98f061402fcf by Serhiy Storchaka in branch 'default':
Issue #20804: The unittest.mock.sentinel attributes now preserve their
https://hg.python.org/cpython/rev/98f061402fcf

New changeset 398b73dbb1a0 by Serhiy Storchaka in branch '3.5':
Issue #20804: Document the limitation of the unittest.mock.sentinel attributes.
https://hg.python.org/cpython/rev/398b73dbb1a0

Thank you for your review Michael.