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*

Except in this case it's only one.

> [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.

That goes in NetworkHandler.fields I believe, which is in one of the later branches in this series. I'll add it there.

> [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?

The same words went through my head, believe me! The problem is that in this case, the object has the expected values and the dict has the tested values.

> [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.

Yeah, I added that in one of the other branches.