-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increased test coverage for serialization #311
Conversation
I think it can be removed as the default_storage_backend isn't a
An ABCMeta metaclass makes calling isinstance much more costly(~10) and we call isinstance with |
hm.. but in principle it's still possible to set the default_storage_backend as a
so I guess, instead of invoking the garbace collector, we can just check whether the object currently stored in the registry (if any) is
-.- |
IMHO, if the user sets it as an WeakValueDict, he is responsible for cleaning up. If desired we could provide tools for this. |
Merge if you think this is ok. |
Closes #294
I looked into coveralls to see which lines/branches are not yet covered by tests.
In particular the case in Serializable.init() where a pulse might get registered only after the gc is invoked was not covered. I tried to do this (delete a registered pulse with del while explicitely disabling the automatic gc) but couldn't cause the desired behavior. Are we sure that the invocation of the gc is necessary at that point?
Also, there's the issue of the AnonymousSerializable. It is currently not neatly integrated into the Serializable class tree and should really be (or be an abstract class defining abstract methods).