From: Thomas Markwalder Date: Tue, 14 May 2019 19:42:34 +0000 (-0400) Subject: [#597,!323] Use round() when calculating T1/T2 to ensure consistent results X-Git-Tag: Kea-1.6.0-beta~153 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2F597-teetest-calculatetimers-unittest-failing;p=thirdparty%2Fkea.git [#597,!323] Use round() when calculating T1/T2 to ensure consistent results src/bin/dhcp4/dhcp4_srv.cc Dhcpv4Srv::setTeeTimes() - added use of round() src/bin/dhcp6/dhcp6_srv.cc Dhcpv6Srv::setTeeTimes() - added use of round() src/bin/dhcp6/tests/dhcp6_test_utils.cc Dhcpv6SrvTest::checkIA_NA() - replaced used of EXPECTED_EQ with logic to add a failure so callers can ASSERT_NO_FATAL_FAILURE and pinpoint the invocation. src/bin/dhcp6/tests/hooks_unittest.cc Wrapped calls to checkIA_NA() with ASSERT_NO_FATAL_FAILURE Updated expected T1 values to adjust for use of round(). --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 9f37271225..a04976c871 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -2269,7 +2269,7 @@ Dhcpv4Srv::setTeeTimes(const Lease4Ptr& lease, const Subnet4Ptr& subnet, Pkt4Ptr t2_time = subnet->getT2(); } else if (subnet->getCalculateTeeTimes()) { // Calculating tee times is enabled, so calculated it. - t2_time = static_cast(subnet->getT2Percent() * (lease->valid_lft_)); + t2_time = static_cast(round(subnet->getT2Percent() * (lease->valid_lft_))); } // Send the T2 candidate value only if it's sane: to be sane it must be less than @@ -2288,7 +2288,7 @@ Dhcpv4Srv::setTeeTimes(const Lease4Ptr& lease, const Subnet4Ptr& subnet, Pkt4Ptr t1_time = subnet->getT1(); } else if (subnet->getCalculateTeeTimes()) { // Calculating tee times is enabled, so calculate it. - t1_time = static_cast(subnet->getT1Percent() * (lease->valid_lft_)); + t1_time = static_cast(round(subnet->getT1Percent() * (lease->valid_lft_))); } // Send T1 if it's sane: If we sent T2, T1 must be less than that. If not it must be diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 607ca76406..37ad8b9ffa 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -3908,7 +3908,7 @@ Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6 t2_time = subnet->getT2(); } else if (subnet->getCalculateTeeTimes()) { // Calculating tee times is enabled, so calculate it. - t2_time = static_cast(subnet->getT2Percent() * preferred_lft); + t2_time = static_cast(round(subnet->getT2Percent() * preferred_lft)); } // We allow T2 to be any value. @@ -3922,7 +3922,7 @@ Dhcpv6Srv::setTeeTimes(uint32_t preferred_lft, const Subnet6Ptr& subnet, Option6 t1_time = subnet->getT1(); } else if (subnet->getCalculateTeeTimes()) { // Calculating tee times is enabled, so calculate it. - t1_time = static_cast(subnet->getT1Percent() * preferred_lft); + t1_time = static_cast(round(subnet->getT1Percent() * preferred_lft)); } // T1 is sane if it is less than or equal to T2. diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index f25e164586..3891f2aef5 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -96,9 +96,23 @@ Dhcpv6SrvTest::checkIA_NA(const Pkt6Ptr& rsp, uint32_t expected_iaid, return (boost::shared_ptr()); } - EXPECT_EQ(expected_iaid, ia->getIAID()); - EXPECT_EQ(expected_t1, ia->getT1()); - EXPECT_EQ(expected_t2, ia->getT2()); + if (expected_iaid != ia->getIAID()) { + ADD_FAILURE() << "ia->iaid: " << ia->getIAID() + << " is not expected value: " << expected_iaid; + return (boost::shared_ptr()); + } + + if (expected_t1 != ia->getT1()) { + ADD_FAILURE() << "ia->t1: " << ia->getT1() + << " is not expected value: " << expected_t1; + return (boost::shared_ptr()); + } + + if (expected_t2 != ia->getT2()) { + ADD_FAILURE() << "ia->t2: " << ia->getT2() + << " is not expected value: " << expected_t2; + return (boost::shared_ptr()); + } tmp = ia->getOption(D6O_IAADDR); boost::shared_ptr addr = boost::dynamic_pointer_cast(tmp); diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index 3a92dfbfd0..e28c00baf9 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -2741,8 +2741,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Renew) { ASSERT_TRUE(tmp); // Check that IA_NA was returned and that there's an address included - boost::shared_ptr addr_opt = checkIA_NA(reply, 234, subnet_->getT1(), - subnet_->getT2()); + boost::shared_ptr addr_opt; + ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 234, subnet_->getT1(), + subnet_->getT2())); ASSERT_TRUE(addr_opt); // Check that the lease is really in the database @@ -2834,7 +2835,8 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Renew) { ASSERT_TRUE(tmp); // Check that IA_NA was returned and that there's an address included - boost::shared_ptr addr_opt = checkIA_NA(reply, 1000, 601, 802); + boost::shared_ptr addr_opt; + ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 1000, 602, 802)); ASSERT_TRUE(addr_opt); // Check that the lease is really in the database @@ -3863,8 +3865,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Rebind) { ASSERT_TRUE(tmp); // Check that IA_NA was returned and that there's an address included - boost::shared_ptr addr_opt = checkIA_NA(reply, 234, subnet_->getT1(), - subnet_->getT2()); + boost::shared_ptr addr_opt; + ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 234, subnet_->getT1(), + subnet_->getT2())); ASSERT_TRUE(addr_opt); // Check that the lease is really in the database @@ -3951,7 +3954,8 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Rebind) { // Check that IA_NA was returned and that there's an address included // Note we also verify that T1 and T2 were calculated correctly. - boost::shared_ptr addr_opt = checkIA_NA(reply, 1000, 601, 802); + boost::shared_ptr addr_opt; + ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(reply, 1000, 602, 802)); ASSERT_TRUE(addr_opt); // Check that the lease is really in the database @@ -4757,7 +4761,8 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier) { ASSERT_TRUE(tmp); // Check that IA_NA was returned and that there's an address included - boost::shared_ptr addr_opt = checkIA_NA(adv, 234, 1000, 2000); + boost::shared_ptr addr_opt; + ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(adv, 234, 1000, 2000)); ASSERT_TRUE(addr_opt); ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText()); @@ -4837,7 +4842,8 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier_hwaddr) { ASSERT_TRUE(tmp); // Check that IA_NA was returned and that there's an address included - boost::shared_ptr addr_opt = checkIA_NA(adv, 234, 1000, 2000); + boost::shared_ptr addr_opt; + ASSERT_NO_FATAL_FAILURE(addr_opt = checkIA_NA(adv, 234, 1000, 2000)); ASSERT_TRUE(addr_opt); ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());