From: Marcin Siodelski <marcin@isc.org>
Date: Tue, 5 Jun 2018 07:52:19 +0000 (+0200)
Subject: [5638] Added commenary to simpleAlloc tests.
X-Git-Tag: trac5117_base~1^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e8ed8fe0fddfb454544e1fea9a35be111d0b85ca;p=thirdparty%2Fkea.git

[5638] Added commenary to simpleAlloc tests.

As a result of review.
---

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index f46ca30c05..72e7d70896 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -47,7 +47,16 @@ TEST_F(AllocEngine4Test, constructor) {
     EXPECT_THROW(x->getAllocator(Lease::TYPE_PD), BadValue);
 }
 
-// This test checks if the simple IPv4 allocation can succeed
+// This test checks if two simple IPv4 allocations succeed and that the
+// statistics is properly updated. Prior to the second allocation it
+// resets the pointer to the last allocated address within the address
+// pool. This causes the engine to walk over the already allocated
+// address and then pick the first available address for the second
+// allocation. Because the allocation engine checks the callouts next
+// step status after each attempt to allocate an address, this test
+// also sets this status to non-default value prior to the second
+// allocation attempt, to make sure that this unexpected status will
+// not interfere with the allocation.
 TEST_F(AllocEngine4Test, simpleAlloc4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
@@ -81,6 +90,7 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
     // Assigned addresses should have incremented.
     EXPECT_TRUE(testStatistics("assigned-addresses", 1, subnet_->getID()));
 
+    // Second allocation starts here.
     uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
     HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     AllocEngine::ClientContext4 ctx2(subnet_, ClientIdPtr(), hwaddr2, IOAddress("0.0.0.0"),
@@ -88,12 +98,18 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
                                      false);
     ctx2.query_.reset(new Pkt4(DHCPREQUEST, 1234));
 
+    // Set the next step to non-default value to verify that it doesn't
+    // affect the allocation.
     ctx2.callout_handle_ = HooksManager::createCalloutHandle();
     ctx2.callout_handle_->setStatus(CalloutHandle::NEXT_STEP_SKIP);
 
+    // Set the last allocated to the beginning of the pool. The allocation
+    // engine should detect that the first address is already allocated and
+    // assign the first available one.
     pool_->resetLastAllocated();
 
     lease = engine->allocateLease4(ctx2);
+
     // The new lease has been allocated, so the old lease should not exist.
     EXPECT_FALSE(ctx2.old_lease_);
 
@@ -106,6 +122,9 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
 
     // Now check that the lease in LeaseMgr has the same parameters
     detailCompareLease(lease, from_mgr);
+
+    // Assigned addresses should have incremented.
+    EXPECT_TRUE(testStatistics("assigned-addresses", 2, subnet_->getID()));
 }
 
 // This test checks if the fake allocation (for DHCPDISCOVER) can succeed
diff --git a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
index e4baabdfce..8394fd79be 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine6_unittest.cc
@@ -67,8 +67,16 @@ TEST_F(AllocEngine6Test, constructor) {
     EXPECT_THROW(x->getAllocator(Lease::TYPE_V4), BadValue);
 }
 
-// This test checks if the simple allocation (REQUEST) can succeed
-// and the stats counter is properly bumped by 1
+// This test checks if two simple IPv6 allocations succeed and that the
+// statistics is properly updated. Prior to the second allocation it
+// resets the pointer to the last allocated address within the address
+// pool. This causes the engine to walk over the already allocated
+// address and then pick the first available address for the second
+// allocation. Because the allocation engine checks the callouts next
+// step status after each attempt to allocate an address, this test
+// also sets this status to non-default value prior to the second
+// allocation attempt, to make sure that this unexpected status will
+// not interfere with the allocation.
 TEST_F(AllocEngine6Test, simpleAlloc6) {
     // Assigned count should be zero.
     EXPECT_TRUE(testStatistics("assigned-nas", 0, subnet_->getID()));