> This code is very difficult to review since its complexity has gone through
> the roof.
>
> I think I said this the last this was changed, but I really think we should
> break this down into multiple queries (via views, perhaps) so that individual
> pieces can be tested easier.
>
> For now, I attempted to review by comparing the added/changed tests to the
> added/changed code. I think I would like to see more specific tests to capture
> each edge case, such as:
>
> test__default_domain_is_extra_special_ipv4
> test__default_domain_is_extra_special_ipv6
These are both covered via:
test_user_reserved_addresses_included_in_get_hostname_ip_mapping
test_user_reserved_addresses_included_in_correct_domains
test_user_reserved_IP_on_node_or_default_domain_not_both
I have updated them to be more clear about what they are expecting to see.
> test__domains_are_special
This is covered (among others) via:
test_user_reserved_addresses_included_in_correct_domains
> test__attached_addresses_only_map_back_to_node
This is covered via:
test_get_hostname_ip_mapping_handles_dnsrr_node_collision
> That way it's clearer to future maintainers how each edge case and branch
> called out in the SQL maps to a requirement for a specific behavior.
> This code is very difficult to review since its complexity has gone through domain_ is_extra_ special_ ipv4 domain_ is_extra_ special_ ipv6 user_reserved_ addresses_ included_ in_get_ hostname_ ip_mapping user_reserved_ addresses_ included_ in_correct_ domains user_reserved_ IP_on_node_ or_default_ domain_ not_both
> the roof.
>
> I think I said this the last this was changed, but I really think we should
> break this down into multiple queries (via views, perhaps) so that individual
> pieces can be tested easier.
>
> For now, I attempted to review by comparing the added/changed tests to the
> added/changed code. I think I would like to see more specific tests to capture
> each edge case, such as:
>
> test__default_
> test__default_
These are both covered via:
test_
test_
test_
I have updated them to be more clear about what they are expecting to see.
> test__domains_ are_special user_reserved_ addresses_ included_ in_correct_ domains
This is covered (among others) via:
test_
> test__attached_ addresses_ only_map_ back_to_ node get_hostname_ ip_mapping_ handles_ dnsrr_node_ collision
This is covered via:
test_
> That way it's clearer to future maintainers how each edge case and branch
> called out in the SQL maps to a requirement for a specific behavior.