Port some Xapi unit tests from OUnit to Alcotest framework

ocaml/tests/suite.ml

@@ -20,24 +20,18 @@ let base_suite =
     Test_helpers.test;
     Test_datamodel_utils.test;
     Test_http.test;
-    Test_pool_db_backup.test;
-    Test_xapi_db_upgrade.test;
     Test_ha_vm_failover.test;
     Test_map_check.test;
     Test_pool_license.test;
     Test_features.test;
-    Test_pool_restore_database.test;
     Test_platformdata.test;
     Test_sm_features.test;
-    Test_pci_helpers.test;
     Test_vgpu_type.test;
-    Test_pgpu.test;
     Test_pgpu_helpers.test;
     Test_storage_migrate_state.test;
     Test_vm.test;
     Test_vm_helpers.test;
     Test_xenopsd_metadata.test;
-    Test_workload_balancing.test;
     Test_cpuid_helpers.test;
     Test_pool_cpuinfo.test;
     (* Test_ca121350.test; *)
@@ -53,7 +47,6 @@ let base_suite =
     Test_network_sriov.test;
     Test_xapi_vbd_helpers.test;
     Test_sr_update_vdis.test;
-    Test_network.test;
     Test_host_helpers.test;
   ]
 

ocaml/tests/suite_alcotest.ml

@@ -7,23 +7,30 @@ let () =
 
   Alcotest.run "Base suite"
     [ "Test_valid_ref_list", Test_valid_ref_list.test
+    ; "Test_pci_helpers", Test_pci_helpers.test
     ; "Test_vdi_allowed_operations", Test_vdi_allowed_operations.test
     ; "Test_vm_migrate", Test_vm_migrate.test
     ; "Test_no_migrate", Test_no_migrate.test
     ; "Test_vm_check_operation_error", Test_vm_check_operation_error.test
     ; "Test_host", Test_host.test
     ; "Test_vdi_cbt", Test_vdi_cbt.test
+    ; "Test_xapi_db_upgrade", Test_xapi_db_upgrade.test
     ; "Test_db_lowlevel", Test_db_lowlevel.test
     ; "Test_vlan", Test_vlan.test
+    ; "Test_network", Test_network.test
     ; "Test_agility", Test_agility.test
     ; "Test_daemon_manager", Test_daemon_manager.test
     ; "Test_cluster", Test_cluster.test
     ; "Test_cluster_host", Test_cluster_host.test
     ; "Test_client", Test_client.test
     ; "Test_ca91480", Test_ca91480.test
+    ; "Test_pgpu", Test_pgpu.test
     ; "Test_gpu_group", Test_gpu_group.test
     ; "Test_pool_apply_edition", Test_pool_apply_edition.test
     ; "Test_pool_update", Test_pool_update.test
+    ; "Test_pool_db_backup", Test_pool_db_backup.test
+    ; "Test_pool_restore_database", Test_pool_restore_database.test
+    ; "Test_workload_balancing", Test_workload_balancing.test
     ; "Test_pusb", Test_pusb.test
     ; "Test_pvs_site", Test_pvs_site.test
     ; "Test_pvs_proxy", Test_pvs_proxy.test

ocaml/tests/test_network.ml

@@ -1,138 +1,132 @@
 
-let test_purpose_setters =
-  let assert_equal ~msg p1 p2 = Ounit_comparators.NetworkPurposeSet.(assert_equal ~msg (of_list p1) (of_list p2)) in
-
-  let with_test f =
-    let __context = Mock.make_context_with_new_db "Mock context" in
-    let network ~purpose = Test_common.make_network ~__context ~purpose () in
-    let add_purpose = Xapi_network.add_purpose ~__context in
-    let remove_purpose = Xapi_network.remove_purpose ~__context in
-    let get_purpose = Db.Network.get_purpose ~__context in
-    f network add_purpose remove_purpose get_purpose
-  in
-
-  let test_add_purpose () =
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        let _network2 : _ API.Ref.t = network ~purpose:[] in
-        add_purpose ~self:network1 ~value:`nbd;
-        assert_equal
-          ~msg:"Should be able to add 'nbd' purpose when none of the networks have a purpose"
-          [`nbd]
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        let _network2 : _ API.Ref.t = network ~purpose:[`nbd] in
-        add_purpose ~self:network1 ~value:`nbd;
-        assert_equal
-          ~msg:"Should be able to add 'nbd' purpose when the other network has the same purpose"
-          [`nbd]
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        let _network2 : _ API.Ref.t = network ~purpose:[] in
-        add_purpose ~self:network1 ~value:`insecure_nbd;
-        assert_equal
-          ~msg:"Should be able to add 'insecure_nbd' purpose when none of the networks have a purpose"
-          [`insecure_nbd]
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        let _network2 : _ API.Ref.t = network ~purpose:[`insecure_nbd] in
-        add_purpose ~self:network1 ~value:`insecure_nbd;
-        assert_equal
-          ~msg:"Should be able to add 'insecure_nbd' purpose when the other network has the same purpose"
-          [`insecure_nbd]
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[`nbd] in
-        add_purpose ~self:network1 ~value:`nbd;
-        assert_equal
-          ~msg:"add_purpose should be idempotent and should do nothing when adding an existing purpose"
-          [`nbd]
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[`nbd] in
-        OUnit.assert_raises
-          ~msg:"Should not be allowed to add 'insecure_nbd' to a network that already has the 'nbd' purpose"
-          (* The first parameter of this error is the new purpose we're trying to add, the second is the conflicting one. *)
-          Api_errors.(Server_error (network_incompatible_purposes, ["insecure_nbd"; "nbd"]))
-          (fun () -> add_purpose ~self:network1 ~value:`insecure_nbd);
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        let _network2 : _ API.Ref.t = network ~purpose:[`nbd] in
-        OUnit.assert_raises
-          ~msg:"Should not be allowed to add 'insecure_nbd' when another network already has the 'nbd' purpose"
-          Api_errors.(Server_error (network_incompatible_purposes, ["insecure_nbd"; "nbd"]))
-          (fun () -> add_purpose ~self:network1 ~value:`insecure_nbd);
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[`insecure_nbd] in
-        OUnit.assert_raises
-          ~msg:"Should not be allowed to add 'nbd' to a network that already has the 'insecure_nbd' purpose"
-          Api_errors.(Server_error (network_incompatible_purposes, ["nbd"; "insecure_nbd"]))
-          (fun () -> add_purpose ~self:network1 ~value:`nbd);
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        let _network2 : _ API.Ref.t = network ~purpose:[`insecure_nbd] in
-        OUnit.assert_raises
-          ~msg:"Should not be allowed to add 'nbd' when another network already has the 'insecure_nbd' purpose"
-          Api_errors.(Server_error (network_incompatible_purposes, ["nbd"; "insecure_nbd"]))
-          (fun () -> add_purpose ~self:network1 ~value:`nbd);
-      )
-  in
-
-  let test_remove_purpose () =
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[`nbd] in
-        remove_purpose ~self:network1 ~value:`insecure_nbd;
-        assert_equal
-          ~msg:"remove_purpose should be idempotent and should do nothing when removing an invalid purpose that is not present"
-          [`nbd]
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[] in
-        remove_purpose ~self:network1 ~value:`nbd;
-        assert_equal
-          ~msg:"remove_purpose should be idempotent and should do nothing when removing a valid purpose that isn't present"
-          []
-          (get_purpose ~self:network1)
-      );
-
-    with_test (fun network add_purpose remove_purpose get_purpose ->
-        let network1 = network ~purpose:[`nbd] in
-        remove_purpose ~self:network1 ~value:`nbd;
-        assert_equal
-          ~msg:"remove_purpose should successfully remove a purpose that is present"
-          []
-          (get_purpose ~self:network1)
-      )
-  in
-
-  let ((>:::), (>::)) = OUnit.((>:::), (>::)) in
-  "test_purpose_setters" >:::
-  [ "test_add_purpose" >:: test_add_purpose
-  ; "test_remove_purpose" >:: test_remove_purpose
-  ]
+let assert_equal msg p1 p2 =
+  let network_purpose = Alcotest_comparators.from_rpc_of_t API.rpc_of_network_purpose in
+  Alcotest.(check (slist network_purpose compare))
+    msg
+    p1 p2
+
+let with_test f =
+  let __context = Mock.make_context_with_new_db "Mock context" in
+  let network ~purpose = Test_common.make_network ~__context ~purpose () in
+  let add_purpose = Xapi_network.add_purpose ~__context in
+  let remove_purpose = Xapi_network.remove_purpose ~__context in
+  let get_purpose = Db.Network.get_purpose ~__context in
+  f network add_purpose remove_purpose get_purpose
+
+let test_add_purpose () =
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      let _network2 : _ API.Ref.t = network ~purpose:[] in
+      add_purpose ~self:network1 ~value:`nbd;
+      assert_equal
+        "Should be able to add 'nbd' purpose when none of the networks have a purpose"
+        [`nbd]
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      let _network2 : _ API.Ref.t = network ~purpose:[`nbd] in
+      add_purpose ~self:network1 ~value:`nbd;
+      assert_equal
+        "Should be able to add 'nbd' purpose when the other network has the same purpose"
+        [`nbd]
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      let _network2 : _ API.Ref.t = network ~purpose:[] in
+      add_purpose ~self:network1 ~value:`insecure_nbd;
+      assert_equal
+        "Should be able to add 'insecure_nbd' purpose when none of the networks have a purpose"
+        [`insecure_nbd]
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      let _network2 : _ API.Ref.t = network ~purpose:[`insecure_nbd] in
+      add_purpose ~self:network1 ~value:`insecure_nbd;
+      assert_equal
+        "Should be able to add 'insecure_nbd' purpose when the other network has the same purpose"
+        [`insecure_nbd]
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[`nbd] in
+      add_purpose ~self:network1 ~value:`nbd;
+      assert_equal
+        "add_purpose should be idempotent and should do nothing when adding an existing purpose"
+        [`nbd]
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[`nbd] in
+      Alcotest.check_raises
+        "Should not be allowed to add 'insecure_nbd' to a network that already has the 'nbd' purpose"
+        (* The first parameter of this error is the new purpose we're trying to add, the second is the conflicting one. *)
+        Api_errors.(Server_error (network_incompatible_purposes, ["insecure_nbd"; "nbd"]))
+        (fun () -> add_purpose ~self:network1 ~value:`insecure_nbd);
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      let _network2 : _ API.Ref.t = network ~purpose:[`nbd] in
+      Alcotest.check_raises
+        "Should not be allowed to add 'insecure_nbd' when another network already has the 'nbd' purpose"
+        Api_errors.(Server_error (network_incompatible_purposes, ["insecure_nbd"; "nbd"]))
+        (fun () -> add_purpose ~self:network1 ~value:`insecure_nbd);
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[`insecure_nbd] in
+      Alcotest.check_raises
+        "Should not be allowed to add 'nbd' to a network that already has the 'insecure_nbd' purpose"
+        Api_errors.(Server_error (network_incompatible_purposes, ["nbd"; "insecure_nbd"]))
+        (fun () -> add_purpose ~self:network1 ~value:`nbd);
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      let _network2 : _ API.Ref.t = network ~purpose:[`insecure_nbd] in
+      Alcotest.check_raises
+        "Should not be allowed to add 'nbd' when another network already has the 'insecure_nbd' purpose"
+        Api_errors.(Server_error (network_incompatible_purposes, ["nbd"; "insecure_nbd"]))
+        (fun () -> add_purpose ~self:network1 ~value:`nbd);
+    )
+
+let test_remove_purpose () =
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[`nbd] in
+      remove_purpose ~self:network1 ~value:`insecure_nbd;
+      assert_equal
+        "remove_purpose should be idempotent and should do nothing when removing an invalid purpose that is not present"
+        [`nbd]
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[] in
+      remove_purpose ~self:network1 ~value:`nbd;
+      assert_equal
+        "remove_purpose should be idempotent and should do nothing when removing a valid purpose that isn't present"
+        []
+        (get_purpose ~self:network1)
+    );
+
+  with_test (fun network add_purpose remove_purpose get_purpose ->
+      let network1 = network ~purpose:[`nbd] in
+      remove_purpose ~self:network1 ~value:`nbd;
+      assert_equal
+        "remove_purpose should successfully remove a purpose that is present"
+        []
+        (get_purpose ~self:network1)
+    )
 
 let test =
-  let ((>:::), (>::)) = OUnit.((>:::), (>::)) in
-  "test_network" >:::
-  [ test_purpose_setters ]
+  [ "test_add_purpose", `Quick, test_add_purpose
+  ; "test_remove_purpose", `Quick, test_remove_purpose
+  ]

ocaml/tests/test_pci_helpers.ml

@@ -12,39 +12,35 @@
  * GNU Lesser General Public License for more details.
  *)
 
-open OUnit
-open Xapi_pci_helpers
 
-(* This test generates a lot of print --- set skip to false to enable *)
-let skip = true
+(* This test generates a lot of print which will now be output to a separate file *)
 
-let print_host_pcis () =
-  skip_if skip "Generates lots of text...";
+let test_print_host_pcis () =
   try
     print_string "===== Host PCIs =====\n\n";
-    let pcis = get_host_pcis () in
-    List.iter
-      (fun p ->
-         let x_to_str = Printf.sprintf "%04x" in
-         Printf.printf "%s " (String.concat " "
-                                [
-                                  p.address;
-                                  x_to_str p.vendor.id;
-                                  p.vendor.name;
-                                  x_to_str p.device.id;
-                                  p.device.name;
-                                  x_to_str p.pci_class.id;
-                                  p.pci_class.name
-                                ]);
-         List.iter (fun s -> print_string (s ^ ", ")) p.related;
-         print_newline ())
-      pcis
+    let pcis = Xapi_pci_helpers.get_host_pcis () in
+    Alcotest.(check unit)
+      "test_pci_helpers"
+      ()
+      (List.iter
+        Xapi_pci_helpers.(fun p ->
+           let x_to_str = Printf.sprintf "%04x" in
+           Printf.printf "%s " (String.concat " "
+                                  [
+                                    p.address;
+                                    x_to_str p.vendor.id;
+                                    p.vendor.name;
+                                    x_to_str p.device.id;
+                                    p.device.name;
+                                    x_to_str p.pci_class.id;
+                                    p.pci_class.name
+                                  ]);
+           List.iter (fun s -> print_string (s ^ ", ")) p.related;
+           print_newline ())
+        pcis)
   with e ->
-    print_string (Printexc.to_string e);
-    assert_equal 0 1
+    Alcotest.fail (Printexc.to_string e)
 
 let test =
-  "test_pci_helpers" >:::
-  [
-    "print_host_pcis" >:: print_host_pcis;
+  [ "test_print_host_pcis", `Quick, test_print_host_pcis
   ]