CCI-MOC / hil

Hardware Isolation Layer, formerly Hardware as a Service
Apache License 2.0
24 stars 54 forks source link

show_headnode test suite failure #512

Closed zenhack closed 8 years ago

zenhack commented 8 years ago
___________________________________________________________________________ TestQuery.test_show_headnode ___________________________________________________________________________
self = <api.TestQuery instance at 0x7f727cf78560>, db = <sqlalchemy.orm.session.Session object at 0x7f727cf51c10>

    def test_show_headnode(self, db):
        api.project_create('anvil-nextgen')
        network_create_simple('spiderwebs', 'anvil-nextgen')
        api.headnode_create('BGH', 'anvil-nextgen', 'base-headnode')
        api.headnode_create_hnic('BGH', 'eth0')
        api.headnode_create_hnic('BGH', 'wlan0')
        api.headnode_connect_network('BGH', 'eth0', 'spiderwebs')

        result = json.loads(api.show_headnode('BGH'))
        # For the lists to be equal, the ordering must be the same:
        result['hnics'].sort()
>       assert result == {
            'name': 'BGH',
            'project': 'anvil-nextgen',
            'hnics': [
                'eth0',
                'wlan0',
            ],
            'vncport': None
        }
E       assert {'base_imgs':...nextgen', ...} == {'hnics': ['et...ncport': None}
E         Omitting 4 identical items, use -v to show
E         Left contains more items:
E         {u'base_imgs': u'base-headnode',
E          u'uuid': u'bce1ed76-b948-11e5-b2b0-001e65327848'}

tests/unit/api.py:1335: AssertionError

This looks like it was due to a recent change. About that CI...

Also, what was the reasoning behind putting the uuid in the output? It was originally just there as an implementation detail, to make sure that the libvirt names were unique; exposing it as part of the API doesn't make a lot of sense to me.

henn commented 8 years ago

Thanks for your report! We are working on getting CI (#90).

RE: show the uuid, the idea was that folks were wanting to do operations on the VMs (snapshot, clone, etc), but had to go digging in the DB to find out which corresponded to their head node.

zenhack commented 8 years ago

Fair enough. my take on that though would be to change things such that the names were more self-identifying -- you have no lookup step that way, and you keep the API simpler. Maybe just do some escaping of the label and use that (I think the reasoning for UUIDs was that it was simpler than properly escaping the names, but it's not that hard). Having something in there that only exists so you can do stuff behind the curtains seems a bit ugly to me. Just my $0.02.

henn commented 8 years ago

Let's continue the unit test discussion in #517