From af61e847ce5199c292570e13cc695ed164cb8421 Mon Sep 17 00:00:00 2001 From: IWASE Yusuke Date: Thu, 25 Jan 2018 15:45:26 +0900 Subject: scenario_test: Enable to try assertion several times Some times, on Travis-CI, some test cases fail unexpectedly in checking paths in RIBs due to advertisements are not yet received from other routers. Currently, in order to avoid this unexpected result, "time.sleep" is inserted after adding new routes, but it is not enough. This patch introduces a new function to enable to do assertion several times and avoid failure with the first assertion. Note: This patch does not introduces this change into all test cases, do only into some test cases which fail relatively frequently. Signed-off-by: IWASE Yusuke --- test/lib/base.py | 12 ++ test/scenario_test/addpath_test.py | 201 ++++++++++++++++++++-------------- test/scenario_test/aspath_test.py | 40 +++++-- test/scenario_test/bgp_router_test.py | 32 +++--- 4 files changed, 175 insertions(+), 110 deletions(-) (limited to 'test') diff --git a/test/lib/base.py b/test/lib/base.py index c7afba6d..657c9623 100644 --- a/test/lib/base.py +++ b/test/lib/base.py @@ -123,6 +123,18 @@ def try_several_times(f, t=3, s=1): raise e +def assert_several_times(f, t=30, s=1): + e = AssertionError + for _ in range(t): + try: + f() + except AssertionError as e: + time.sleep(s) + else: + return + raise e + + def get_bridges(): return try_several_times(lambda: local("docker network ls | awk 'NR > 1{print $2}'", capture=True)).split('\n') diff --git a/test/scenario_test/addpath_test.py b/test/scenario_test/addpath_test.py index de0f337b..2d71554b 100644 --- a/test/scenario_test/addpath_test.py +++ b/test/scenario_test/addpath_test.py @@ -21,7 +21,10 @@ import nose from fabric.api import local from lib import base -from lib.base import BGP_FSM_ESTABLISHED +from lib.base import ( + BGP_FSM_ESTABLISHED, + assert_several_times, +) from lib.gobgp import GoBGPContainer from lib.exabgp import ExaBGPContainer from lib.noseplugin import OptionParser, parser_option @@ -74,148 +77,176 @@ class GoBGPTestBase(unittest.TestCase): self.e1.add_route(route='192.168.100.0/24', identifier=10, aspath=[100, 200, 300]) self.e1.add_route(route='192.168.100.0/24', identifier=20, aspath=[100, 200]) self.e1.add_route(route='192.168.100.0/24', identifier=30, aspath=[100]) - # Because ExaBGPContainer will be restarted internally for adding or - # deleting routes, here waits for re-establishment. - self.g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.e1) - time.sleep(1) # test three routes are installed to the rib due to add-path feature def test_02_check_g1_global_rib(self): - rib = self.g1.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 3) + def f(): + rib = self.g1.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 3) + + assert_several_times(f) # test only the best path is advertised to g2 def test_03_check_g2_global_rib(self): - rib = self.g2.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 1) - self.assertEqual(rib[0]['paths'][0]['aspath'], [100]) + def f(): + rib = self.g2.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 1) + self.assertEqual(rib[0]['paths'][0]['aspath'], [100]) + + assert_several_times(f) # test three routes are advertised to g3 def test_04_check_g3_global_rib(self): - rib = self.g3.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 3) + def f(): + rib = self.g3.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 3) + + assert_several_times(f) # withdraw a route with path_id (no error check) def test_05_withdraw_route_with_path_id(self): self.e1.del_route(route='192.168.100.0/24', identifier=30) - # Because ExaBGPContainer will be restarted internally for adding or - # deleting routes, here waits for re-establishment. - self.g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.e1) - time.sleep(1) # test the withdrawn route is removed from the rib def test_06_check_g1_global_rib(self): - rib = self.g1.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 2) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200]) + def f(): + rib = self.g1.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 2) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200])) + + assert_several_times(f) # test the best path is replaced due to the removal from g1 rib def test_07_check_g2_global_rib(self): - rib = self.g2.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 1) - self.assertEqual(rib[0]['paths'][0]['aspath'], [100, 200]) + def f(): + rib = self.g2.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 1) + self.assertEqual(rib[0]['paths'][0]['aspath'], [100, 200]) + + assert_several_times(f) # test the withdrawn route is removed from the rib of g3 def test_08_check_g3_global_rib(self): - rib = self.g3.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 2) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200]) + def f(): + rib = self.g3.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 2) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200])) + + assert_several_times(f) # install a route with path_id via GoBGP CLI (no error check) def test_09_install_add_paths_route_via_cli(self): # identifier is duplicated with the identifier of the route from e1 self.g1.add_route(route='192.168.100.0/24', identifier=10, local_pref=500) - time.sleep(1) # XXX: wait for routes re-calculated and advertised # test the route from CLI is installed to the rib def test_10_check_g1_global_rib(self): - rib = self.g1.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 3) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200] or - path['aspath'] == []) - if len(path['aspath']) == 0: - self.assertEqual(path['local-pref'], 500) + def f(): + rib = self.g1.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 3) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200], + [])) + if not path['aspath']: # path['aspath'] == [] + self.assertEqual(path['local-pref'], 500) + + assert_several_times(f) # test the best path is replaced due to the CLI route from g1 rib def test_11_check_g2_global_rib(self): - rib = self.g2.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 1) - self.assertEqual(rib[0]['paths'][0]['aspath'], []) + def f(): + rib = self.g2.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 1) + self.assertEqual(rib[0]['paths'][0]['aspath'], []) + + assert_several_times(f) # test the route from CLI is advertised from g1 def test_12_check_g3_global_rib(self): - rib = self.g3.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 3) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200] or - path['aspath'] == []) - if len(path['aspath']) == 0: - self.assertEqual(path['local-pref'], 500) + def f(): + rib = self.g3.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 3) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200], + [])) + if not path['aspath']: # path['aspath'] == [] + self.assertEqual(path['local-pref'], 500) + + assert_several_times(f) # remove non-existing route with path_id via GoBGP CLI (no error check) def test_13_remove_non_existing_add_paths_route_via_cli(self): # specify locally non-existing identifier which has the same value # with the identifier of the route from e1 self.g1.del_route(route='192.168.100.0/24', identifier=20) - time.sleep(1) # XXX: wait for routes re-calculated and advertised # test none of route is removed by non-existing path_id via CLI def test_14_check_g1_global_rib(self): - rib = self.g1.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 3) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200] or - path['aspath'] == []) - if len(path['aspath']) == 0: - self.assertEqual(path['local-pref'], 500) + def f(): + rib = self.g1.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 3) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200], + [])) + if not path['aspath']: # path['aspath'] == [] + self.assertEqual(path['local-pref'], 500) + + assert_several_times(f) # remove route with path_id via GoBGP CLI (no error check) def test_15_remove_add_paths_route_via_cli(self): self.g1.del_route(route='192.168.100.0/24', identifier=10) - time.sleep(1) # XXX: wait for routes re-calculated and advertised # test the route is removed from the rib via CLI def test_16_check_g1_global_rib(self): - rib = self.g1.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 2) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200]) + def f(): + rib = self.g1.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 2) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200])) + + assert_several_times(f) # test the best path is replaced the removal from g1 rib def test_17_check_g2_global_rib(self): - rib = self.g2.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 1) - self.assertEqual(rib[0]['paths'][0]['aspath'], [100, 200]) + def f(): + rib = self.g2.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 1) + self.assertEqual(rib[0]['paths'][0]['aspath'], [100, 200]) + + assert_several_times(f) # test the removed route from CLI is withdrawn by g1 def test_18_check_g3_global_rib(self): - rib = self.g3.get_global_rib() - self.assertEqual(len(rib), 1) - self.assertEqual(len(rib[0]['paths']), 2) - for path in rib[0]['paths']: - self.assertTrue(path['aspath'] == [100, 200, 300] or - path['aspath'] == [100, 200]) + def f(): + rib = self.g3.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 2) + for path in rib[0]['paths']: + self.assertIn(path['aspath'], ([100, 200, 300], + [100, 200])) + + assert_several_times(f) if __name__ == '__main__': diff --git a/test/scenario_test/aspath_test.py b/test/scenario_test/aspath_test.py index f6087c46..5f6b42a8 100644 --- a/test/scenario_test/aspath_test.py +++ b/test/scenario_test/aspath_test.py @@ -25,7 +25,10 @@ import nose from lib.noseplugin import OptionParser, parser_option from lib import base -from lib.base import BGP_FSM_ESTABLISHED +from lib.base import ( + BGP_FSM_ESTABLISHED, + assert_several_times, +) from lib.gobgp import GoBGPContainer from lib.quagga import QuaggaBGPContainer @@ -69,8 +72,10 @@ class GoBGPTestBase(unittest.TestCase): self.q1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.g2) def test_02_check_reject_as_loop(self): - time.sleep(1) - self.assertTrue(len(self.g2.get_global_rib()) == 0) + def f(): + self.assertEqual(len(self.g2.get_global_rib()), 0) + + assert_several_times(f) def test_03_update_peer(self): self.g2.update_peer(self.q1, allow_as_in=10) @@ -78,8 +83,10 @@ class GoBGPTestBase(unittest.TestCase): self.q1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=self.g2) def test_04_check_accept_as_loop(self): - time.sleep(1) - self.assertTrue(len(self.g2.get_global_rib()) == 1) + def f(): + self.assertEqual(len(self.g2.get_global_rib()), 1) + + assert_several_times(f) def test_05_check_remove_private_as_peer_all(self): g3 = GoBGPContainer(name='g3', asn=100, router_id='192.168.0.4', @@ -102,8 +109,13 @@ class GoBGPTestBase(unittest.TestCase): self.g2.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g3) g3.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g4) - time.sleep(1) - self.assertTrue(g4.get_global_rib()[0]['paths'][0]['aspath'] == [100]) + def f(): + rib = g4.get_global_rib() + self.assertEqual(len(rib), 1) + self.assertEqual(len(rib[0]['paths']), 1) + self.assertEqual(rib[0]['paths'][0]['aspath'], [100]) + + assert_several_times(f) def test_06_check_remove_private_as_peer_replace(self): g3 = self.ctns['g3'] @@ -112,8 +124,11 @@ class GoBGPTestBase(unittest.TestCase): g3.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g4) - time.sleep(1) - self.assertTrue(g4.get_global_rib()[0]['paths'][0]['aspath'] == [100, 100, 100, 100]) + def f(): + rib = g4.get_global_rib() + self.assertEqual(rib[0]['paths'][0]['aspath'], [100, 100, 100, 100]) + + assert_several_times(f) def test_07_check_replace_peer_as(self): g5 = GoBGPContainer(name='g5', asn=100, router_id='192.168.0.6', @@ -127,8 +142,11 @@ class GoBGPTestBase(unittest.TestCase): g4.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=g5) - time.sleep(1) - self.assertTrue(g5.get_global_rib()[0]['paths'][0]['aspath'] == [200, 200, 200, 200, 200]) + def f(): + rib = g5.get_global_rib() + self.assertEqual(rib[0]['paths'][0]['aspath'], [200, 200, 200, 200, 200]) + + assert_several_times(f) if __name__ == '__main__': diff --git a/test/scenario_test/bgp_router_test.py b/test/scenario_test/bgp_router_test.py index 7d93ba53..727c598e 100644 --- a/test/scenario_test/bgp_router_test.py +++ b/test/scenario_test/bgp_router_test.py @@ -33,6 +33,7 @@ from lib.base import ( BGP_ATTR_TYPE_MULTI_EXIT_DISC, BGP_ATTR_TYPE_LOCAL_PREF, wait_for_completion, + assert_several_times, ) from lib.gobgp import ( GoBGPContainer, @@ -312,17 +313,17 @@ class GoBGPTestBase(unittest.TestCase): self.test_02_check_gobgp_global_rib() paths = q1.get_global_rib('20.0.0.0/24') - self.assertTrue(len(paths) == 1) + self.assertEqual(len(paths), 1) n_addrs = [i[1].split('/')[0] for i in self.gobgp.ip_addrs] - self.assertTrue(paths[0]['nexthop'] in n_addrs) + self.assertIn(paths[0]['nexthop'], n_addrs) q3.stop() - time.sleep(3) + self.gobgp.wait_for(expected_state=BGP_FSM_ACTIVE, peer=q3) paths = q1.get_global_rib('20.0.0.0/24') - self.assertTrue(len(paths) == 1) - self.assertTrue(paths[0]['nexthop'] in n_addrs) + self.assertEqual(len(paths), 1) + self.assertIn(paths[0]['nexthop'], n_addrs) g1.del_peer(q3) del self.quaggas['q3'] @@ -340,19 +341,22 @@ class GoBGPTestBase(unittest.TestCase): self.test_02_check_gobgp_global_rib() paths = g1.get_adj_rib_out(q1, '30.0.0.0/24') - self.assertTrue(len(paths) == 1) - self.assertTrue('source-id' not in paths[0]) + self.assertEqual(len(paths), 1) + self.assertNotIn('source-id', paths[0]) paths = g1.get_adj_rib_out(q2, '30.0.0.0/24') - self.assertTrue(len(paths) == 1) - self.assertTrue('source-id' not in paths[0]) + self.assertEqual(len(paths), 1) + self.assertNotIn('source-id', paths[0]) g1.local('gobgp global rib del 30.0.0.0/24') - paths = g1.get_adj_rib_out(q1, '30.0.0.0/24') - self.assertTrue(len(paths) == 0) - paths = g1.get_adj_rib_out(q2, '30.0.0.0/24') - self.assertTrue(len(paths) == 1) - self.assertTrue(paths[0]['source-id'] == '192.168.0.2') + def f(): + paths = g1.get_adj_rib_out(q1, '30.0.0.0/24') + self.assertEqual(len(paths), 0) + paths = g1.get_adj_rib_out(q2, '30.0.0.0/24') + self.assertEqual(len(paths), 1) + self.assertEqual(paths[0]['source-id'], '192.168.0.2') + + assert_several_times(f) def test_19_check_grpc_add_neighbor(self): g1 = self.gobgp -- cgit v1.2.3