robertc at robertcollins.net
Tue May 9 01:10:23 BST 2006
On Tue, 2006-05-09 at 00:24 +0200, Jan Hudec wrote:
> I already reported on the way InventoryLink.symlink_target is used in
> InventoryEntry and works because the other entries don't have __slots__. Now
> I've found a related issue.
> There is a test, test_inv.TestInventoryEntry.test_dir_detect_changes, that
> def test_dir_detect_changes(self):
> left = inventory.InventoryDirectory('123', 'hello.c', ROOT_ID)
> left.text_sha1 = 123
> left.executable = True
> In other words, it sets 3 parameters that InventoryDirectory should not have
> at all (which works because InventoryEntry does not declare __slots__, so it
> can have attributes added) and checks whether they are ignored.
> It also happens further down the line where InventoryLink entry get's it's
> executable flag set (and is tested whether it ignores it).
> But shouldn't it rather be an error to set these attributes?
The code that uses InventoryEntry was completely unprepared for
subclasses, but could not be cleaned until there were subclasses.... The
test tests that the API used by the calling code was preserved intact
during the introduction of subclasses. The migration probably is not
complete : there is possibly still code that assumes any InventoryEntry
is of the base type and can handle any attribute.
The test should be removed after an audit that all uses of
InventoryEntry either create a subclass directory, or are given an
instance from somewhere else and only use subclass specific features
indirectly - either invoking a method on the base class that is
overridden, or by doing a (yeuch) type code check.
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060509/297a63a6/attachment.pgp
More information about the bazaar