Add RouteTarget and VRF factories. Add TenancyFilterTestCaseMixin and refactor

[glennmatthews]

Towards: #2283

What's Changed

• Added factories for RouteTarget, VRF, Role, VLANGroup, and VLAN models and add these factories to the test runner.
• Added OrganizationalModelFactory and PrimaryModelFactory base classes - currently these are mostly empty but I've left TODO comments regarding potential areas for extension of these. I moved the Tag mapping logic out of the individual factories and into PrimaryModelFactory so that it doesn't need to be repeated everywhere.
  • Also added BaseModelFactory as a common parent of these two classes, and overrode the _create API (which despite the leading underscore is a documented extension point) to call our .validated_save() method when creating model instances to ensure they are valid.
• Added get_random_instances factory helper function, and used it for Tag mapping as well as import_target/export_target mapping on the VRF factory.
• As a result of the factory updates, I discovered a bug in the filtering test updates for tenant_group filters that I made in Factory-boy fixtures: Tenant, TenantGroup, RIR, Aggregate #2479, specifically that the tests weren't accounting for the fact that tenant_group is a TreeNodeMultipleChoice filter and so matches not only against the specific tenant group(s) provided, but also any of their descendants. As a result many of the test_tenant_group tests began failing after I modified the factories (and hence the specific set of random data being generated) because they weren't accounting for the possibility of sub-groups.
  • Testing for tenant and tenant_group filters is basically boilerplate, and I got tired of making the same change in a dozen different places, so I went ahead and created a TenancyFilterTestCaseMixin class and made use of it throughout our tests. This results in a decent amount of test code reduction, yay!

TODO

• Explanation of Change(s)
• Attached Screenshots, Payload Example
• Unit, Integration Tests
• Documentation Updates (when adding/changing features)
• n/a Example Plugin Updates (when adding/changing features)
• Outline Remaining Work, Constraints from Design

[bryanculver]

This is looking pretty good. I'd like to understand a little bit more the need for two separate random instance getters.

[glennmatthews]

This is looking pretty good. I'd like to understand a little bit more the need for two separate random instance getters.

One is for getting a single random instance (e.g., ForeignKey), the other is for getting 0-to-n random instances (e.g., ManyToManyField). The latter is "new" in this PR but only inasmuch as it's a generalization of the get_random_tags_for_model function that was introduced in #2479. (Which I just realized is still left in the code at the moment, even though it's no longer used - will get that fixed in the next commit.)

[glennmatthews]

One change I want to draw attention to is the change in the various test_filter cases from:

self.assertEqual(self.filterset(params, self.queryset).qs.count(), HARDCODED_NUMBER)

to:

self.assertQuerysetEqual(self.filterset(params, self.queryset), self.queryset.filter(...))

First, it accounts for the fact that due to factories we don't necessarily have a specific number of filtered instances known in advance. But secondly, using assertQuerysetEqual instead of just assertEqual(count, count) makes for a stronger test IMHO.

In some cases, the queryset ordering may not necessarily be guaranteed, such as in the VRF case where we might have two VRFs with the same name and a null rd value, the relative ordering of which would not be guaranteed. In these cases (only) we need to pass ordered=False to the assertQuerysetEqual() call.

[gsnider2195]

Why was distinct required here?

[glennmatthews]

In general it seems that querysets using a reverse lookup, e.g. queryset.filter(<related_name>__in=...) can return duplicate entries. Not sure if every single case where I added distinct() is needed but at least some of them definitely were.

[jathanism]

I only have the TODO questions, but nothing blocking. This is looking pretty solid. Another @glennmatthews hit.