Code review comment for lp:~jason-hobbs/maas/ucs-xml-api

This is half a review. I have to go out, and I'll finish it later.

As for splitting it up, for one you could have added an empty add_ucsm()
function to NodeGroup, building the API calls around it, and writing
tests in which you mock the behaviour of add_ucsm().

[1]

Hope-quoting, as Julian has mentioned.

[2]

+    def add_ucsm(self, url, username, password):

This isn't a great name for this method. However, I see that it's
consistent with what's gone before.

[3]

+        children = map(Element, children_tags)
+        request_data = make_request_data('foo', fields, children)

Bear in mind that map() is lazy in Python 3. Will make_request_data()
deal with that? If not, make it explicit with a list comprehension.

[4]

+        self.assertEqual(set(children_tags), set([e.tag for e in root[:]]))

The list-comprehension is not needed:

        self.assertEqual(set(children_tags), set(e.tag for e in root[:]))

Not sure the slice is either?

        self.assertEqual(set(children_tags), set(e.tag for e in root))

Having said that, assertItemsEqual() is useful here:

        self.assertItemsEqual(children_tags, (e.tag for e in root))

[5]

+        self.assertRaises(UCSM_XML_API_Error, lambda: parse_response(xml))

assertRaises() allows arguments, so you can write this as:

        self.assertRaises(UCSM_XML_API_Error, parse_response, xml)

[6]

+    def make_test_api(self, url='http://url', user='u', password='p',
+                      cookie='foo', mock_call=True):
+
+        api = make_test_api(url, user, password, cookie)
+
+        if mock_call:
+            mock = self.patch(api, '_call')
+        else:
+            mock = None
+
+        return api, mock

Boolean arguments are a bit smelly, as is returning irrelevant
things. Can you split this into two methods?

    def make_test_api(
            self, url='http://url', user='u', password='p', cookie='foo'):
        return make_test_api(url, user, password, cookie)

    def make_test_api(
            self, url='http://url', user='u', password='p', cookie='foo'):
        api = make_test_api(url, user, password, cookie)
        return api, self.patch(api, '_call')

To be continued...