From 4a03ab11fb3d0a6fc2fb08c897fd47d79f465f5a Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Mon, 4 Feb 2019 18:22:01 -0600 Subject: [PATCH] #1093 properly send in hostId when creating a dedicated host VSI --- CONTRIBUTING.md | 65 +++++++++++++++++++++++++ SoftLayer/managers/vs.py | 4 +- tests/CLI/modules/vs/vs_create_tests.py | 9 +++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0def27891..1eed6d308 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,5 +35,70 @@ Docs are generated with [Sphinx](https://docs.readthedocs.io/en/latest/intro/get `make html` in the softlayer-python/docs directory, which should generate the HTML in softlayer-python/docs/_build/html for testing. +## Unit Tests +All new features should be 100% code covered, and your pull request should at the very least increase total code overage. +### Mocks +To tests results from the API, we keep mock results in SoftLayer/fixtures// with the method name matching the variable name. + +Any call to a service that doesn't have a fixture will result in a TransportError + +### Overriding Fixtures + +Adding your expected output in the fixtures file with a unique name is a good way to define a fixture that gets used frequently in a test. + +```python +from SoftLayer.fixtures import SoftLayer_Product_Package + + def test_test(self): + amock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + amock.return_value = fixtures.SoftLayer_Product_Package.RESERVED_CAPACITY +``` + +Otherwise defining it on the spot works too. +```python + def test_test(self): + mock = self.set_mock('SoftLayer_Network_Storage', 'getObject') + mock.return_value = { + 'billingItem': {'hourlyFlag': True, 'id': 449}, + } +``` + + +### Call testing +Testing your code to make sure it makes the correct API call is also very important. + +The testing.TestCase class has a method call `assert_called_with` which is pretty handy here. + +```python +self.assert_called_with( + 'SoftLayer_Billing_Item', # Service + 'cancelItem', # Method + args=(True, True, ''), # Args + identifier=449, # Id + mask=mock.ANY, # object Mask, + filter=mock.ANY, # object Filter + limit=0, # result Limit + offset=0 # result Offset +) +``` + +Making sure a API was NOT called + +```python +self.assertEqual([], self.calls('SoftLayer_Account', 'getObject')) +``` + +Making sure an API call has a specific arg, but you don't want to list out the entire API call (like with a place order test) + +```python +# Get the API Call signature +order_call = self.calls('SoftLayer_Product_Order', 'placeOrder') + +# Get the args property of that API call, which is a tuple, with the first entry being our data. +order_args = getattr(order_call[0], 'args')[0] + +# Test our specific argument value +self.assertEqual(123, order_args['hostId']) +``` \ No newline at end of file diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index 0f5a6d26a..a3f26126e 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -910,9 +910,11 @@ def order_guest(self, guest_object, test=False): if guest_object.get('userdata'): # SL_Virtual_Guest::generateOrderTemplate() doesn't respect userData, so we need to add it ourself template['virtualGuests'][0]['userData'] = [{"value": guest_object.get('userdata')}] - + if guest_object.get('host_id'): + template['hostId'] = guest_object.get('host_id') if guest_object.get('placement_id'): template['virtualGuests'][0]['placementGroupId'] = guest_object.get('placement_id') + if test: result = self.client.call('Product_Order', 'verifyOrder', template) else: diff --git a/tests/CLI/modules/vs/vs_create_tests.py b/tests/CLI/modules/vs/vs_create_tests.py index 61fe1cee5..5075d225e 100644 --- a/tests/CLI/modules/vs/vs_create_tests.py +++ b/tests/CLI/modules/vs/vs_create_tests.py @@ -294,8 +294,13 @@ def test_create_with_host_id(self, confirm_mock): self.assert_no_fail(result) self.assertIn('"guid": "1a2b3c-1701"', result.output) + # Argument testing Example + order_call = self.calls('SoftLayer_Product_Order', 'placeOrder') + order_args = getattr(order_call[0], 'args')[0] + self.assertEqual(123, order_args['hostId']) + self.assert_called_with('SoftLayer_Product_Order', 'placeOrder') - args = ({ + template_args = ({ 'startCpus': 2, 'maxMemory': 1024, 'hostname': 'host', @@ -319,7 +324,7 @@ def test_create_with_host_id(self, confirm_mock): ] },) - self.assert_called_with('SoftLayer_Virtual_Guest', 'generateOrderTemplate', args=args) + self.assert_called_with('SoftLayer_Virtual_Guest', 'generateOrderTemplate', args=template_args) @mock.patch('SoftLayer.CLI.formatting.confirm') def test_create_like(self, confirm_mock):