From f075339c5f3e7d21ec88d82766a6791d48e5b91c Mon Sep 17 00:00:00 2001 From: Sander Steffann Date: Tue, 16 Jun 2020 21:48:26 +0200 Subject: [PATCH] Improve test comments and remove over-enthusiastic tests --- netbox/dcim/tests/test_models.py | 160 ++++++++++++++----------------- 1 file changed, 73 insertions(+), 87 deletions(-) diff --git a/netbox/dcim/tests/test_models.py b/netbox/dcim/tests/test_models.py index f3b890102..c55d099c9 100644 --- a/netbox/dcim/tests/test_models.py +++ b/netbox/dcim/tests/test_models.py @@ -383,6 +383,10 @@ class CableTestCase(TestCase): self.front_port3 = FrontPort.objects.create( device=self.patch_pannel, name='FP3', type='8p8c', rear_port=self.rear_port3, rear_port_position=1 ) + self.rear_port4 = RearPort.objects.create(device=self.patch_pannel, name='RP4', type='8p8c', positions=3) + self.front_port4 = FrontPort.objects.create( + device=self.patch_pannel, name='FP4', type='8p8c', rear_port=self.rear_port4, rear_port_position=1 + ) self.provider = Provider.objects.create(name='Provider 1', slug='provider-1') self.circuittype = CircuitType.objects.create(name='Circuit Type 1', slug='circuit-type-1') self.circuit = Circuit.objects.create(provider=self.provider, type=self.circuittype, cid='1') @@ -453,10 +457,21 @@ class CableTestCase(TestCase): with self.assertRaises(ValidationError): cable.clean() - def test_singlepos_rearport_connections(self): + def test_connection_via_single_position_rearport(self): """ - A RearPort with one position can be connected to anything as it is just a - cable extender. + A RearPort with one position can be connected to anything. + + [CableTermination X]---[RP(pos=1) FP]---[CableTermination Y] + + is allowed anywhere + + [CableTermination X]---[CableTermination Y] + + is allowed. + + A RearPort with multiple positions may not be directly connected to a path endpoint or another RearPort + with a different number of positions. RearPorts with a single position on the other hand may be connected + to such CableTerminations. Check that this is indeed allowed. """ # Connecting a single-position RearPort to a multi-position RearPort is ok Cable(termination_a=self.rear_port1, termination_b=self.rear_port2).full_clean() @@ -464,11 +479,59 @@ class CableTestCase(TestCase): # Connecting a single-position RearPort to an Interface is ok Cable(termination_a=self.rear_port1, termination_b=self.interface3).full_clean() - def test_multipos_rearport_connections(self): + # Connecting a single-position RearPort to a CircuitTermination is ok + Cable(termination_a=self.rear_port1, termination_b=self.circuittermination1).full_clean() + + def test_connection_via_multi_position_rearport(self): """ - A RearPort with more than one position can only be connected to another RearPort with the same number of - positions. + A RearPort with multiple positions may not be directly connected to a path endpoint or another RearPort + with a different number of positions. + + The following scenario's are allowed (with x>1): + + ~----------+ +---------~ + | | + RP2(pos=x)|---|RP(pos=x) + | | + ~----------+ +---------~ + + ~----------+ +---------~ + | | + RP2(pos=x)|---|RP(pos=1) + | | + ~----------+ +---------~ + + ~----------+ +------------------~ + | | + RP2(pos=x)|---|CircuitTermination + | | + ~----------+ +------------------~ + + These scenarios are NOT allowed (with x>1): + + ~----------+ +----------~ + | | + RP2(pos=x)|---|RP(pos!=x) + | | + ~----------+ +----------~ + + ~----------+ +----------~ + | | + RP2(pos=x)|---|Interface + | | + ~----------+ +----------~ + + These scenarios are tested in this order below. """ + # Connecting a multi-position RearPort to another RearPort with the same number of positions is ok + Cable(termination_a=self.rear_port3, termination_b=self.rear_port4).full_clean() + + # Connecting a multi-position RearPort to a single-position RearPort is ok + Cable(termination_a=self.rear_port2, termination_b=self.rear_port1).full_clean() + + # Connecting a multi-position RearPort to a CircuitTermination is ok + Cable(termination_a=self.rear_port2, termination_b=self.circuittermination1).full_clean() + with self.assertRaises( ValidationError, msg='Connecting a 2-position RearPort to a 3-position RearPort should fail' @@ -481,10 +544,7 @@ class CableTestCase(TestCase): ): Cable(termination_a=self.rear_port2, termination_b=self.interface3).full_clean() - # Connecting a multi-position RearPort to a CircuitTermination should be ok - Cable(termination_a=self.rear_port1, termination_b=self.circuittermination1).full_clean() - - def test_cable_cannot_terminate_to_a_virtual_inteface(self): + def test_cable_cannot_terminate_to_a_virtual_interface(self): """ A cable cannot terminate to a virtual interface """ @@ -493,7 +553,7 @@ class CableTestCase(TestCase): with self.assertRaises(ValidationError): cable.clean() - def test_cable_cannot_terminate_to_a_wireless_inteface(self): + def test_cable_cannot_terminate_to_a_wireless_interface(self): """ A cable cannot terminate to a wireless interface """ @@ -604,7 +664,7 @@ class CablePathTestCase(TestCase): self.assertIsNone(endpoint_a.connection_status) self.assertIsNone(endpoint_b.connection_status) - def test_connection_via_one_on_one_port(self): + def test_connection_via_single_rear_port(self): """ Test a connection which passes through a rear port with exactly one front port. @@ -650,38 +710,7 @@ class CablePathTestCase(TestCase): self.assertIsNone(endpoint_a.connection_status) self.assertIsNone(endpoint_b.connection_status) - # Recreate cable 1 to test creating the cables in reverse order (RP first, FP second) - cable1 = Cable( - termination_a=Interface.objects.get(device__name='Device 1', name='Interface 1'), - termination_b=FrontPort.objects.get(device__name='Panel 5', name='Front Port 1') - ) - cable1.full_clean() - cable1.save() - - # Refresh endpoints - endpoint_a.refresh_from_db() - endpoint_b.refresh_from_db() - - # Validate connections - self.assertEqual(endpoint_a.connected_endpoint, endpoint_b) - self.assertEqual(endpoint_b.connected_endpoint, endpoint_a) - self.assertTrue(endpoint_a.connection_status) - self.assertTrue(endpoint_b.connection_status) - - # Delete cable 2 - cable2.delete() - - # Refresh endpoints - endpoint_a.refresh_from_db() - endpoint_b.refresh_from_db() - - # Check that connections have been nullified - self.assertIsNone(endpoint_a.connected_endpoint) - self.assertIsNone(endpoint_b.connected_endpoint) - self.assertIsNone(endpoint_a.connection_status) - self.assertIsNone(endpoint_b.connection_status) - - def test_connection_via_nested_one_on_one_port(self): + def test_connections_via_nested_single_position_rearport(self): """ Test a connection which passes through a single front/rear port pair between two multi-position rear ports. @@ -772,49 +801,6 @@ class CablePathTestCase(TestCase): self.assertIsNone(endpoint_c.connection_status) self.assertIsNone(endpoint_d.connection_status) - # Recreate cable 3 to test reverse order (Panel 5 FP first, RP second) - cable3 = Cable( - termination_b=RearPort.objects.get(device__name='Panel 1', name='Rear Port 1'), - termination_a=RearPort.objects.get(device__name='Panel 5', name='Rear Port 1') - ) - cable3.full_clean() - cable3.save() - - # Refresh endpoints - endpoint_a.refresh_from_db() - endpoint_b.refresh_from_db() - endpoint_c.refresh_from_db() - endpoint_d.refresh_from_db() - - # Validate connections - self.assertEqual(endpoint_a.connected_endpoint, endpoint_b) - self.assertEqual(endpoint_b.connected_endpoint, endpoint_a) - self.assertEqual(endpoint_c.connected_endpoint, endpoint_d) - self.assertEqual(endpoint_d.connected_endpoint, endpoint_c) - self.assertTrue(endpoint_a.connection_status) - self.assertTrue(endpoint_b.connection_status) - self.assertTrue(endpoint_c.connection_status) - self.assertTrue(endpoint_d.connection_status) - - # Delete cable 4 - cable4.delete() - - # Refresh endpoints - endpoint_a.refresh_from_db() - endpoint_b.refresh_from_db() - endpoint_c.refresh_from_db() - endpoint_d.refresh_from_db() - - # Check that connections have been nullified - self.assertIsNone(endpoint_a.connected_endpoint) - self.assertIsNone(endpoint_b.connected_endpoint) - self.assertIsNone(endpoint_c.connected_endpoint) - self.assertIsNone(endpoint_d.connected_endpoint) - self.assertIsNone(endpoint_a.connection_status) - self.assertIsNone(endpoint_b.connection_status) - self.assertIsNone(endpoint_c.connection_status) - self.assertIsNone(endpoint_d.connection_status) - def test_connections_via_patch(self): """ Test two connections via patched rear ports: