From 1d42c05de14f1a3b4935e7bef1f957292112aa38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 2 Oct 2024 11:54:30 +0100 Subject: [PATCH 1/2] test(xapi_globs): add a unit test that xapi globs parsing works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We had a bug that would not fail any unit tests, but XAPI would fail to start This reproduces the error now: ``` [invalid] compare: functional value Raised by primitive operation at Xapi_globs.other_options.(fun) in file "ocaml/xapi/xapi_globs.ml", line 1622, characters 17-59 Called from Xcp_service.Config_file.dump.(fun) in file "ocaml/xapi-idl/lib/xcp_service.ml", line 145, characters 34-46 Called from Stdlib__List.iter in file "list.ml", line 110, characters 12-15 Called from Xcp_service.configure_common in file "ocaml/xapi-idl/lib/xcp_service.ml", line 432, characters 2-30 Called from Xcp_service.configure in file "ocaml/xapi-idl/lib/xcp_service.ml", line 461, characters 4-259 Called from Alcotest_engine__Core.Make.protect_test.(fun) in file "src/alcotest-engine/core.ml", line 181, characters 17-23 Called from Alcotest_engine__Monad.Identity.catch in file "src/alcotest-engine/monad.ml", line 24, characters 31-35 ``` Also print the Failure message in Xcp_service.configure, otherwise it'd exit 1 without saying why if you supply invalid cmdline arguments. Signed-off-by: Edwin Török --- ocaml/tests/test_xapi_helpers.ml | 11 ++++++++++- ocaml/xapi-idl/lib/xcp_service.ml | 6 +++--- ocaml/xapi-idl/lib/xcp_service.mli | 3 ++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ocaml/tests/test_xapi_helpers.ml b/ocaml/tests/test_xapi_helpers.ml index 172e5c6e6a1..587a0888f6b 100644 --- a/ocaml/tests/test_xapi_helpers.ml +++ b/ocaml/tests/test_xapi_helpers.ml @@ -40,6 +40,15 @@ let filtering_test = ) strings +let test_xapi_configure () = + Xcp_service.configure + ~argv:[|Sys.argv.(0)|] + ~options:Xapi_globs.all_options () + let () = Suite_init.harness_init () ; - Alcotest.run "Test XAPI Helpers suite" [("Test_xapi_helpers", filtering_test)] + Alcotest.run "Test XAPI Helpers suite" + [ + ("Test_xapi_helpers", filtering_test) + ; ("Test_xapi_configure", [("configure", `Quick, test_xapi_configure)]) + ] diff --git a/ocaml/xapi-idl/lib/xcp_service.ml b/ocaml/xapi-idl/lib/xcp_service.ml index 645b04d0864..a7683091323 100644 --- a/ocaml/xapi-idl/lib/xcp_service.ml +++ b/ocaml/xapi-idl/lib/xcp_service.ml @@ -456,15 +456,15 @@ let configure_common ~options ~resources arg_parse_fn = resources ; Sys.set_signal Sys.sigpipe Sys.Signal_ignore -let configure ?(options = []) ?(resources = []) () = +let configure ?(argv = Sys.argv) ?(options = []) ?(resources = []) () = try configure_common ~options ~resources (fun config_spec -> - Arg.parse + Arg.parse_argv argv (Arg.align (arg_spec config_spec)) (fun _ -> failwith "Invalid argument") (Printf.sprintf "Usage: %s [-config filename]" Sys.argv.(0)) ) - with Failure _ -> exit 1 + with Failure msg -> prerr_endline msg ; flush stderr ; exit 1 let configure2 ~name ~version ~doc ?(options = []) ?(resources = []) () = configure_common ~options ~resources @@ fun config_spec -> diff --git a/ocaml/xapi-idl/lib/xcp_service.mli b/ocaml/xapi-idl/lib/xcp_service.mli index 05196bc03a0..98f35bea528 100644 --- a/ocaml/xapi-idl/lib/xcp_service.mli +++ b/ocaml/xapi-idl/lib/xcp_service.mli @@ -28,7 +28,8 @@ type res = { ; perms: Unix.access_permission list } -val configure : ?options:opt list -> ?resources:res list -> unit -> unit +val configure : + ?argv:string array -> ?options:opt list -> ?resources:res list -> unit -> unit val configure2 : name:string From a3ab2287e91167c4fd5996f7ef62dd560c2fe921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Wed, 2 Oct 2024 11:30:59 +0100 Subject: [PATCH 2/2] fix(xapi)): cannot compare functional values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: 6635a00d68e5 ("CP-49136: Introduce PRNG for generating non-secret UUIDs") Signed-off-by: Edwin Török --- ocaml/xapi/xapi_globs.ml | 2 +- quality-gate.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 1b0d7c9bdd5..29e404fa224 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -1619,7 +1619,7 @@ let other_options = ; ( "use-prng-uuid-gen" (* eventually this'll be the default, except for Sessions *) , Arg.Unit (fun () -> Uuidx.make_default := Uuidx.make_uuid_fast) - , (fun () -> !Uuidx.make_default = Uuidx.make_uuid_fast |> string_of_bool) + , (fun () -> !Uuidx.make_default == Uuidx.make_uuid_fast |> string_of_bool) , "Use PRNG based UUID generator instead of CSPRNG" ) ; ( "reuse-pool-sessions" diff --git a/quality-gate.sh b/quality-gate.sh index 9c3d3c2b5f8..eb2d4daed90 100755 --- a/quality-gate.sh +++ b/quality-gate.sh @@ -40,7 +40,7 @@ mli-files () { } structural-equality () { - N=9 + N=10 EQ=$(git grep -r --count ' == ' -- '**/*.ml' ':!ocaml/sdk-gen/**/*.ml' | cut -d ':' -f 2 | paste -sd+ - | bc) if [ "$EQ" -eq "$N" ]; then echo "OK counted $EQ usages of ' == '"