Skip to content

Convert test_network_event loop_alcotest and avoid calling the firewall script with repeated interfaces #3571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 20, 2018
Merged

Convert test_network_event loop_alcotest and avoid calling the firewall script with repeated interfaces #3571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ca2729
Convert Test_network_event_loop to alcotest
gaborigloi Mar 20, 2018
c897834
Network_event_loop: make sure no duplicated interfaces are passed to …
gaborigloi Mar 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ocaml/tests/suite.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ let base_suite =
Test_network_sriov.test;
Test_xapi_vbd_helpers.test;
Test_sr_update_vdis.test;
Test_network_event_loop.test;
Test_network.test;
Test_host_helpers.test;
]
Expand Down
1 change: 1 addition & 0 deletions ocaml/tests/suite_alcotest.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ let () =
; "Test_event", Test_event.test
; "Test_vm_placement", Test_vm_placement.test
; "Test_vm_memory_constraints", Test_vm_memory_constraints.test
; "Test_network_event_loop", Test_network_event_loop.test
]

15 changes: 7 additions & 8 deletions ocaml/tests/test_network_event_loop.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,18 @@ let test_network_event_loop ~no_nbd_networks_at_start () =
)
in

let param_set = Alcotest.(slist string String.compare) in

let assert_received_params msg expected =
Thread.delay delay;
match !received_params with
| None -> OUnit.assert_failure ("The update_firewall function was not called: " ^ msg)
| Some p ->
Ounit_comparators.StringSet.(assert_equal (of_list expected) (of_list p))
| None -> Alcotest.fail ("The update_firewall function was not called: " ^ msg)
| Some p -> Alcotest.(check param_set) msg expected p
in

let assert_not_called msg () =
Thread.delay delay;
OUnit.assert_equal ~msg:("update_firewall shouldn't have been called: " ^ msg) None !received_params
Alcotest.(check (option param_set)) ("update_firewall shouldn't have been called: " ^ msg) None !received_params
in

let network1 = Test_common.make_network ~__context ~bridge:"bridge1" () in
Expand Down Expand Up @@ -170,8 +171,6 @@ let test_network_event_loop ~no_nbd_networks_at_start () =
assert_received_params "NBD has been enabled on network4" ["bridge4"]

let test =
let ((>:::), (>::)) = OUnit.((>:::), (>::)) in
"test_network_event_loop" >:::
[ "test_network_event_loop_with_no_networks_at_start" >:: (test_network_event_loop ~no_nbd_networks_at_start:true)
; "test_network_event_loop_with_some_networks_at_start" >:: (test_network_event_loop ~no_nbd_networks_at_start:false)
[ "test_network_event_loop_with_no_networks_at_start", `Slow, (test_network_event_loop ~no_nbd_networks_at_start:true)
; "test_network_event_loop_with_some_networks_at_start", `Slow, (test_network_event_loop ~no_nbd_networks_at_start:false)
]
1 change: 1 addition & 0 deletions ocaml/xapi/network_event_loop.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ let _watch_networks_for_nbd_changes __context ~update_firewall ~wait_after_event
pifs
in
let interfaces = List.map (fun network -> Db.Network.get_bridge ~__context ~self:network) allowed_connected_networks in
let interfaces = Xapi_stdext_std.Listext.List.setify interfaces in
let needs_firewall_update = match allowed_interfaces with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change the test behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed this because it failed after I changed the test, and I realized that it might send duplicated interfaces to the firewall script.

| Some allowed_interfaces ->
not (Xapi_stdext_std.Listext.List.set_equiv interfaces allowed_interfaces)
Expand Down