jsatt / mock

Automatically exported from code.google.com/p/mock
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Propertymocks dont appear to stop non-existant properties from being set when using spec_set #218

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a mock using spec_set so that assigning to non-existant attributes 
will fail with an exception
2. Attach a non-existant property to the mock

What is the expected output? What do you see instead?

It should fail with an exception saying the attribute does not exist. What it 
actually does is lets you add any property regardless of whether the class has 
that property or not. This makes it hard for unittests to pick up problems 
where interfaces/properties have changed (like a property rename)

What version of the product are you using? On what operating system?

1.0.1, python 2.7.4, Ubuntu

Please provide any additional information below.

I've just had a problem where the API changed (and an attribute on the sdk got 
renamed) and the mocked version of the sdk that I use in the tests didnt pick 
this up (I'd like them to fail when I try mocking properties & methods on that 
dont exist on the mocked sdk). I'm rewriting the tests to use spec_set but I 
cant seem to get them to work.

If I have a class MyItemClass with a property name and a method run how do I 
mock this in such a way that if a property or method gets renamed in the sdk I 
pick it up in the unittest.

MyItemClass is on the lines of:

class MyItemClass:
  ...

  @property
  def name(self):
    return self._details.name  

  def run(self):
    ... do something ...
    return result
Currently I am trying:

mock_item = mock.MagicMock(spec_set=sdk.MyItemClass)
type(mock_item).name = mock.PropertyMock(return_value="Item title")       
type(mock_item).run = mock.MagicMock(return_value=foobar)
but this doesnt pick up a renaming in the MyItemClass of the property name to 
item_name, it just carries on mocking (and so the tests pass).

Equally, this code (which the docs say you shouldnt do due to the way 
PropertyMocks are stored):

mock_item = mock.MagicMock(spec_set=sdk.MyItemClass)
mock_item.name = mock.PropertyMock(return_value="Item title")       
mock_item.run = mock.MagicMock(return_value=foobar)

appears to pick up the rename (and fail when name is set), but it doesnt appear 
to work in the same way as before (tests start failing because the properties 
arent returning what tests expect, presumably because of the dropping of the 
type()).

Original issue reported on code.google.com by barcoded...@gmail.com on 14 Nov 2013 at 11:32

GoogleCodeExporter commented 8 years ago
Currently I'm working around this using the following code:

def add_property(mock_object, property_name, property_value) :                  

    prop = mock.PropertyMock(return_value=property_value)                        
    setattr(type(mock_object), property_name, prop)                              
    mock_object.attach_mock(prop, property_name) # this fails if the property name isnt set

... but I'm not sure is this is the best way or not

Original comment by barcoded...@gmail.com on 14 Nov 2013 at 3:24

GoogleCodeExporter commented 8 years ago
So the problem is that using spec_set doesn't prevent you setting attributes on 
the type. ProperyMock only works if you set it on the type.

I can't prevent you setting attributes on the type without a lot of complexity. 
A better fix would be to allow you to set a PropertyMock directly on the 
instance and have the mock automatically elevate it to the type.  That would 
then trigger the spec_set logic.

Original comment by fuzzyman on 17 Nov 2013 at 11:53

GoogleCodeExporter commented 8 years ago
Yep, that makes sense. Presumably the attach_mock method is a reasonable 
workaround until then?

Original comment by barcoded...@gmail.com on 18 Nov 2013 at 10:19

GoogleCodeExporter commented 8 years ago
Yup, attach_mock is a good workaround.

Original comment by fuzzyman on 18 Nov 2013 at 12:51