Code review comment for lp:~jtv/maas/api-list-networks

[0]

50 === added file 'src/maasserver/tests/test_api_networks.py'
[...]
83 + def test_list_returns_network(self):

The name of this test is strange. One would expect test_list_returns_network*s*

[1]

50 === added file 'src/maasserver/tests/test_api_networks.py'
[...]
83 + def test_list_returns_network(self):

I put a break point in this test and look:

(Pdb) parsed_result
[{u'description': u'tjBXqFQ7vK', u'ip': u'217.187.192.0', u'_state': u'<django.db.models.base.ModelState object at 0x962acd0>', u'netmask': u'255.255.192.0', u'vlan_tag': None, u'id': 1, u'name': u'6THuCI'}]

Django adds clutter to the objects (I'm talking about the '_state' field) so we need to explicitly list the fields that we want to return.

[2]

50 === added file 'src/maasserver/tests/test_api_networks.py'
[...]
93 + (
94 + original_network.name,
95 + original_network.ip,
96 + original_network.netmask,
97 + original_network.vlan_tag,
98 + original_network.description,
99 + ),
100 + (
101 + returned_network['name'],
102 + returned_network['ip'],
103 + returned_network['netmask'],
104 + returned_network['vlan_tag'],
105 + returned_network['description'],
106 + ))

Couldn't this benefit from self.assertAttributes?

[3]

50 === added file 'src/maasserver/tests/test_api_networks.py'
[...]
93 + (
94 + original_network.name,
95 + original_network.ip,
96 + original_network.netmask,
97 + original_network.vlan_tag,
98 + original_network.description,
99 + ),
100 + (
101 + returned_network['name'],
102 + returned_network['ip'],
103 + returned_network['netmask'],
104 + returned_network['vlan_tag'],
105 + returned_network['description'],
106 + ))

There is a bug: we should also have a 'resource_uri' field.