[WIP] Refactor Nuage enterprise sandbox so that it also returns count values

[aljazkosir]

Test that were using Nuage enterprise sandbox were using hard-coded values to validate test data. Those hard-coded values were moved to Nuage enterprise sandbox.

\cc @miha-plesko

[miha-plesko]

Thanks @aljazkosir that's something @mshriver has requested long ago and we promised to refactor it later. Well, here we are :)

[miha-plesko]

Is there a reason to first create a hash box and then convert into object? Wouldn't it read better if we do object just right away in L10, like this:

box = Namespace()

# Create empty enterprise aka 'sandbox'.
box.enterprise = dict(obj=nuage.create_enterprise())

# Fill the sandbox with some entities.
 # Method `create_child` returns a tuple (object, connection) and we only need object.
box.template = dict(
  obj=box.enterprise.create_child(nuage.vspk.NUDomainTemplate(name=random_name()))[0]
)
box.domain = dict(
  obj=enterprise.create_child(nuage.vspk.NUDomain(name=random_name(), template_id=box.template['obj'].id))[0],
  num_subnets=1,
  num_security_groups=1  
)
...

return box

[aljazkosir]

You are right, I created objects directly into Namespace. I also saved their names and ids, so we won't have to access 'obj' key everytime.

[mshriver]

Coming back to these older comments after having made some new ones. @miha-plesko I think your suggestion is on the right track, but its still not clear to me why the objects are being wrapped in dictionaries at all here. Can box.enterprise and box.template not be objects? Is this a Namespace limitation? If so, let's use something other than Namespace.

[miha-plesko]

@mshriver the idea here was to make it "flat" - the object itself comes directly from Nuage python library which we wanted to only use within this fixture. So we were thinking to create anonymous class (Namespace) and only copy attributes we are interested in later for tests, then drop the object itself.

Another reason why just passing on the object itself didn't seem a good idea was that it uses some fancy caching mechanism which in turn yield wrong values unless correctly handled in the test, which makes the whole thing less readable.

To make the story short: we want to use Nuage python library barely to CREATE some entities. We don't need to READ any of them, because we know exactly what we created, hence the plain simple Namespace. If we wanted to use the real object, then we would have to move it to wrapanapi, which is an overkill in this case, I think :)

Please let me know if that answers your question, we can also have a quick call to talk it out in person.

[miha-plesko]

Does it have to be string? Perhaps we could update ValidateStats mixin to convert value to string?

[aljazkosir]

All the values in the mixin are now converted to strings.

[miha-plesko]

Honestly, I'd prefer if test would do just

sandbox.l2_group['name']

Because test reads a bit strange with this 'obj' thing. I don't mind having 'obj' stored, but since 99% of our tests just accesses 'name' and 'id' I'd add those two keys to the dictionary. Wdyt?

PS: Yes, fixture would be slightly redundant then, but the key aspect IMAO is to have easily usable fixture, not nicest implementation :)

[miha-plesko]

LGTM, thanks!

[aljazkosir]

@mshriver I rebased this PR, should be ready for a review :).

[mshriver]

I don't think this is a good or efficient pattern, get_object_dictionary. It odd to me in general, and combined with the use of argparse.Namespace makes me wonder what you're really trying to solve here. I think collections.namedtuple is more appropriate in this context, as you define the object type and parameter names before using them. Namespace does give you more freedome to arbitrarily add attributes to box.

Back to my point - you've written and called get_object_dictionary to create a dictionary with name and id keys, and then right here after doing so you still directly reference box.enterprise['obj'].name, instead of box.enterprise['name']. Both seem like awkward references, when box.enterprise should be the object.

[mshriver]

I don't think this layer-cake of Namespace/attribute/dictionary/key makes this entity generation very readable. I'll keep reading, but I'm continuing to think this get_object_dictionary layer is a bad pattern.

[mshriver]

The get_object_dictionary method is a bad pattern here, I don't think its something I would merge into integration_tests. Happy to discuss what you're trying to achieve with that method, and find a more suitable way to structure and reference the data.

[dajoRH]

I detected some fixture changes in commit d5b96ce

The global fixture with_nuage_sandbox is used in the following files:

• cfme/tests/networks/nuage/test_events_are_triggered.py
  • test_creating_entities_emits_events
  • test_creating_entities_triggers_targeted_refresh
• cfme/tests/networks/nuage/test_operations.py
  • test_router_add_subnet

The global fixture with_nuage_sandbox_modscope is used in the following files:

• cfme/tests/networks/nuage/test_inventory.py
  • test_tenant_details
  • test_subnet_details
  • test_l2_subnet_details
  • test_network_router_details
  • test_network_port_vm
  • test_network_port_container
  • test_network_port_l2_vm
  • test_network_port_l2_container

Please, consider creating a PRT run against these tests make sure your fixture changes do not break existing usage 😃

[dajoRH]

Would you mind rebasing this Pull Request against latest master, please?
CFME QE Bot