WPRDC / wprdc-etl

MIT License
8 stars 3 forks source link

Upsert tests #37

Closed saylorsd closed 8 years ago

saylorsd commented 8 years ago

Addedmethod=upsert tests for the loaders as well as tested against 409 error codes.

@bsmithgall Please take a look whenever you have a chance Is iterating over the error codes a safe way to do it?

bsmithgall commented 8 years ago

This isn't doing exactly what you think it is -- the self.error_codes aren't being returned by the mock call to requests. If you want to use those standard error codes, you should do something like PropertyMock(side_effect=self.error_codes) instead.

saylorsd commented 8 years ago

@bsmithgall so I forgot to actually iterate over the codes in the update_metadata() tests, but what you mentioned similar to what I'm doing intest_datastore_load_insert_failed()?

bsmithgall commented 8 years ago

Yes, but you will never hit the 200 code, so either replace the list with just error, or replace the whole assignment with the self.errors iterable if that makes sense. I would do the former.

saylorsd commented 8 years ago

@bsmithgall **just adding the @ in case you don't get alerts for every post

Ok. So I tried passing self. errors iterable like so:

self.error_codes = [
    [409, 200],
    [500, 200]
]
...
type(post.return_value).status_code = PropertyMock(side_effect=self.error_codes)

However, it failed to raise a RuntimeError because it was passing a status code of [409, 200] for the upsert call and [500, 200] for the update_meta_data call.

Thanks for your help!

bsmithgall commented 8 years ago

So the difference is that side_effect looks for an iterable and then for each command that you call, it returns that. For example:

my_test = Mock(side_effect=[1,2,3])
my_test()
# 1
my_test()
# 2
my_test()
# 3

Whereas, you could use the return_value instead, which will always return the same value

my_test = Mock(return_value=1)
my_test()
# 1
my_test()
# 1
my_test()
# 1

So I've made a changes to just use the return_value instead of a side_effect

saylorsd commented 8 years ago

Ahhhh. Sounds good. Thanks! I'll merge this then.