From a20f5a84f38634306e6cc43234b769e8b259208f Mon Sep 17 00:00:00 2001 From: Nathan Rebours Date: Tue, 28 May 2024 17:37:52 +0200 Subject: [PATCH] Make -unused-code-warning tests actual tests In their previous form they were not executed as part of the default build or as part of the runtest alias. They relied on a deriving_inline workflow which wasn't great for tests and it lacked a bit of context on what was being tested so I converted them to cram tests. It's a bit harder to read the generated code than it was but the whole setup is not spread across 3 different directories anymore and the tests are now documented. Signed-off-by: Nathan Rebours --- test/deriving_warning/disable/dune | 4 - .../disable/ppx_warning_flags_test.ml | 48 ----- .../disable/ppx_warning_flags_test.mli | 54 ----- .../{ppx/ppx_warning_flags.ml => driver.ml} | 2 + test/deriving_warning/dune | 8 + test/deriving_warning/enable/dune | 4 - .../enable/ppx_warning_flags_test.ml | 37 ---- .../enable/ppx_warning_flags_test.mli | 39 ---- test/deriving_warning/ppx/dune | 6 - .../ppx/ppx_warning_flags.mli | 1 - test/deriving_warning/run.t | 195 ++++++++++++++++++ 11 files changed, 205 insertions(+), 193 deletions(-) delete mode 100644 test/deriving_warning/disable/dune delete mode 100644 test/deriving_warning/disable/ppx_warning_flags_test.ml delete mode 100644 test/deriving_warning/disable/ppx_warning_flags_test.mli rename test/deriving_warning/{ppx/ppx_warning_flags.ml => driver.ml} (98%) create mode 100644 test/deriving_warning/dune delete mode 100644 test/deriving_warning/enable/dune delete mode 100644 test/deriving_warning/enable/ppx_warning_flags_test.ml delete mode 100644 test/deriving_warning/enable/ppx_warning_flags_test.mli delete mode 100644 test/deriving_warning/ppx/dune delete mode 100644 test/deriving_warning/ppx/ppx_warning_flags.mli create mode 100644 test/deriving_warning/run.t diff --git a/test/deriving_warning/disable/dune b/test/deriving_warning/disable/dune deleted file mode 100644 index f5012165a..000000000 --- a/test/deriving_warning/disable/dune +++ /dev/null @@ -1,4 +0,0 @@ -(library - (name ppx_warning_flags_testing_disable) - (preprocess - (pps ppx_warning_flags_testing -unused-code-warnings=false))) diff --git a/test/deriving_warning/disable/ppx_warning_flags_test.ml b/test/deriving_warning/disable/ppx_warning_flags_test.ml deleted file mode 100644 index 2c402460e..000000000 --- a/test/deriving_warning/disable/ppx_warning_flags_test.ml +++ /dev/null @@ -1,48 +0,0 @@ -type t = int [@@deriving_inline zero_do_warn, one_no_warn, two_do_warn] - -let _ = fun (_ : t) -> () - -include struct - [@@@ocaml.warning "-60"] - - module Zero = struct - type t = T0 - end - - let zero = Zero.T0 - let _ = zero -end [@@ocaml.doc "@inline"] - -include struct - [@@@ocaml.warning "-60"] - - module One = struct - type 'a t = T1 of 'a - end - - let one = One.T1 zero - let _ = one -end [@@ocaml.doc "@inline"] - -include struct - [@@@ocaml.warning "-60"] - - module Two = struct - type ('a, 'b) t = T2 of 'a * 'b - end - - let two = Two.T2 (zero, one) - let _ = two -end [@@ocaml.doc "@inline"] - -[@@@end] - -type s = int [@@deriving_inline alias_warn] - -let _ = fun (_ : s) -> () -let unit_one = () -let _ = unit_one -let unit_two = unit_one -let _ = unit_two - -[@@@end] diff --git a/test/deriving_warning/disable/ppx_warning_flags_test.mli b/test/deriving_warning/disable/ppx_warning_flags_test.mli deleted file mode 100644 index 90569f4ba..000000000 --- a/test/deriving_warning/disable/ppx_warning_flags_test.mli +++ /dev/null @@ -1,54 +0,0 @@ -type t [@@deriving_inline zero_do_warn, one_no_warn, two_do_warn] - -include sig - [@@@ocaml.warning "-32-60"] - - module Zero : sig - type t - end - - val zero : Zero.t -end -[@@ocaml.doc "@inline"] - -include sig - [@@@ocaml.warning "-32-60"] - - module One : sig - type 'a t - end - - val one : Zero.t One.t -end -[@@ocaml.doc "@inline"] - -include sig - [@@@ocaml.warning "-32-60"] - - module Two : sig - type ('a, 'b) t - end - - val two : (Zero.t, Zero.t One.t) Two.t -end -[@@ocaml.doc "@inline"] - -[@@@end] - -type s = int [@@deriving_inline alias_warn] - -include sig - [@@@ocaml.warning "-32"] - - val unit_one : unit -end -[@@ocaml.doc "@inline"] - -include sig - [@@@ocaml.warning "-32"] - - val unit_two : unit -end -[@@ocaml.doc "@inline"] - -[@@@end] diff --git a/test/deriving_warning/ppx/ppx_warning_flags.ml b/test/deriving_warning/driver.ml similarity index 98% rename from test/deriving_warning/ppx/ppx_warning_flags.ml rename to test/deriving_warning/driver.ml index 800c9a0f6..1e4c629f1 100644 --- a/test/deriving_warning/ppx/ppx_warning_flags.ml +++ b/test/deriving_warning/driver.ml @@ -93,3 +93,5 @@ let () = (* The derivers are added from right to left *) Deriving.add_alias "alias_warn" [ alias_no_warn; alias_do_warn ] |> Deriving.ignore + +let () = Driver.standalone () diff --git a/test/deriving_warning/dune b/test/deriving_warning/dune new file mode 100644 index 000000000..2fdc6aab0 --- /dev/null +++ b/test/deriving_warning/dune @@ -0,0 +1,8 @@ +(executable + (name driver) + (libraries ppxlib) + (preprocess + (pps ppxlib.metaquot))) + +(cram + (deps driver.exe)) diff --git a/test/deriving_warning/enable/dune b/test/deriving_warning/enable/dune deleted file mode 100644 index be9800654..000000000 --- a/test/deriving_warning/enable/dune +++ /dev/null @@ -1,4 +0,0 @@ -(library - (name ppx_warning_flags_testing_enable) - (preprocess - (pps ppx_warning_flags_testing -unused-code-warnings=true))) diff --git a/test/deriving_warning/enable/ppx_warning_flags_test.ml b/test/deriving_warning/enable/ppx_warning_flags_test.ml deleted file mode 100644 index e1da6d6eb..000000000 --- a/test/deriving_warning/enable/ppx_warning_flags_test.ml +++ /dev/null @@ -1,37 +0,0 @@ -type t = int [@@deriving_inline zero_do_warn, one_no_warn, two_do_warn] - -let _ = fun (_ : t) -> () - -module Zero = struct - type t = T0 -end - -let zero = Zero.T0 - -include struct - [@@@ocaml.warning "-60"] - - module One = struct - type 'a t = T1 of 'a - end - - let one = One.T1 zero - let _ = one -end [@@ocaml.doc "@inline"] - -module Two = struct - type ('a, 'b) t = T2 of 'a * 'b -end - -let two = Two.T2 (zero, one) - -[@@@end] - -type s = int [@@deriving_inline alias_warn] - -let _ = fun (_ : s) -> () -let unit_one = () -let unit_two = unit_one -let _ = unit_two - -[@@@end] diff --git a/test/deriving_warning/enable/ppx_warning_flags_test.mli b/test/deriving_warning/enable/ppx_warning_flags_test.mli deleted file mode 100644 index 276646366..000000000 --- a/test/deriving_warning/enable/ppx_warning_flags_test.mli +++ /dev/null @@ -1,39 +0,0 @@ -type t [@@deriving_inline zero_do_warn, one_no_warn, two_do_warn] - -module Zero : sig - type t -end - -val zero : Zero.t - -include sig - [@@@ocaml.warning "-32-60"] - - module One : sig - type 'a t - end - - val one : Zero.t One.t -end -[@@ocaml.doc "@inline"] - -module Two : sig - type ('a, 'b) t -end - -val two : (Zero.t, Zero.t One.t) Two.t - -[@@@end] - -type s = int [@@deriving_inline alias_warn] - -val unit_one : unit - -include sig - [@@@ocaml.warning "-32"] - - val unit_two : unit -end -[@@ocaml.doc "@inline"] - -[@@@end] diff --git a/test/deriving_warning/ppx/dune b/test/deriving_warning/ppx/dune deleted file mode 100644 index ed8052a4e..000000000 --- a/test/deriving_warning/ppx/dune +++ /dev/null @@ -1,6 +0,0 @@ -(library - (name ppx_warning_flags_testing) - (kind ppx_deriver) - (libraries ppxlib) - (preprocess - (pps ppxlib.metaquot))) diff --git a/test/deriving_warning/ppx/ppx_warning_flags.mli b/test/deriving_warning/ppx/ppx_warning_flags.mli deleted file mode 100644 index 74bb72986..000000000 --- a/test/deriving_warning/ppx/ppx_warning_flags.mli +++ /dev/null @@ -1 +0,0 @@ -(*_ This signature is deliberately empty. *) diff --git a/test/deriving_warning/run.t b/test/deriving_warning/run.t new file mode 100644 index 000000000..818032ac8 --- /dev/null +++ b/test/deriving_warning/run.t @@ -0,0 +1,195 @@ +We have a driver with 4 derivers linked in: +- zero_do_warn +- one_no_warn +- two_do_warn +- alias_warn + +---------------------------------------- + +Let's consider the following ocaml source file using the zero_do_warn deriver + + $ cat > zero_do_warn.ml << EOF + > type t = int [@@deriving zero_do_warn] + > EOF + +Zero_do_warn is registered with unused_code_warning set true meaning it allows +the driver not to silence unused code and unused module warnings if the +-unused-code-warning flag is set to true. + +Let's call the driver with -unused-code-warnings=false: + + $ ./driver.exe -unused-code-warnings=false -impl zero_do_warn.ml + type t = int[@@deriving zero_do_warn] + include struct let _ = fun (_ : t) -> () end[@@ocaml.doc "@inline"][@@merlin.hide + ] + include + struct + [@@@ocaml.warning "-60"] + module Zero = struct type t = + | T0 end + let zero = Zero.T0 + let _ = zero + end[@@ocaml.doc "@inline"][@@merlin.hide ] + +The generated code is wrapped in an include struct ... end to disable unused module +warnings, as expected. The derived value zero is followed by a let _ = zero to +disable unused value warning. + +Now if we use -unused-code-warnings=true: + + $ ./driver.exe -unused-code-warnings=true -impl zero_do_warn.ml + type t = int[@@deriving zero_do_warn] + include struct let _ = fun (_ : t) -> () end[@@ocaml.doc "@inline"][@@merlin.hide + ] + include struct module Zero = struct type t = + | T0 end + let zero = Zero.T0 end[@@ocaml.doc "@inline"][@@merlin.hide ] + +Here the warning silencing was disabled as it was both allowed by the driver +invocation and the deriver itself. No include wrapping, no warning disabled, no +let _. + +Note that this also applies to .mli files. + +Consider: + + $ cat > zero_do_warn.mli << EOF + > type t = int [@@deriving zero_do_warn] + > EOF + +and compare the result of both driver invocations: + + $ ./driver.exe -unused-code-warnings=false -intf zero_do_warn.mli + type t = int[@@deriving zero_do_warn] + include + sig + [@@@ocaml.warning "-32-60"] + module Zero : sig type t end + val zero : Zero.t + end[@@ocaml.doc "@inline"][@@merlin.hide ] + + $ ./driver.exe -unused-code-warnings=true -intf zero_do_warn.mli + type t = int[@@deriving zero_do_warn] + include sig module Zero : sig type t end val zero : Zero.t end[@@ocaml.doc + "@inline"] + [@@merlin.hide ] + +----------------------------------------------- + +Let's consider the following ocaml source file using the one_no_warn deriver + + $ cat > one_no_warn.ml << EOF + > type t = int [@@deriving one_no_warn] + > EOF + +One_no_warn is registered with unused_code_warning set to false, meaning the driver +should always disable warnings for the generated code, regardless of the value of +the -unused-code-warning flag. The following driver invocations have the same +output: + + $ ./driver.exe -unused-code-warnings=false -impl one_no_warn.ml + type t = int[@@deriving one_no_warn] + include + struct + [@@@ocaml.warning "-60"] + let _ = fun (_ : t) -> () + module One = struct type 'a t = + | T1 of 'a end + let one = One.T1 zero + let _ = one + end[@@ocaml.doc "@inline"][@@merlin.hide ] + + $ ./driver.exe -unused-code-warnings=true -impl one_no_warn.ml + type t = int[@@deriving one_no_warn] + include + struct + [@@@ocaml.warning "-60"] + let _ = fun (_ : t) -> () + module One = struct type 'a t = + | T1 of 'a end + let one = One.T1 zero + let _ = one + end[@@ocaml.doc "@inline"][@@merlin.hide ] + +Same goes for .mli files: + + $ cat > one_no_warn.mli << EOF + > type t = int [@@deriving one_no_warn] + > EOF + + $ ./driver.exe -unused-code-warnings=false -intf one_no_warn.mli + type t = int[@@deriving one_no_warn] + include + sig + [@@@ocaml.warning "-32-60"] + module One : sig type 'a t end + val one : Zero.t One.t + end[@@ocaml.doc "@inline"][@@merlin.hide ] + + $ ./driver.exe -unused-code-warnings=true -intf one_no_warn.mli + type t = int[@@deriving one_no_warn] + include + sig + [@@@ocaml.warning "-32-60"] + module One : sig type 'a t end + val one : Zero.t One.t + end[@@ocaml.doc "@inline"][@@merlin.hide ] + +------------------------------------------------- + +The alias_warn deriver is in fact an alias for two derivers: +- alias_do_warn, which is registered with unused_code_warnings=true +and derives a single `unit_one : unit` value +- alias_no_warn, which is registered with unused_code_warnings=false +and derives a single `unit_two : unit` value + +For the following code: + + $ cat > alias_warn.ml << EOF + > type t = int [@@deriving alias_warn] + > EOF + +We expect that the driver will do the right thing and disable the warning only for +unit_one: + + $ ./driver.exe -unused-code-warnings=true -impl alias_warn.ml + type t = int[@@deriving alias_warn] + include struct let _ = fun (_ : t) -> () end[@@ocaml.doc "@inline"][@@merlin.hide + ] + include struct let unit_one = () end[@@ocaml.doc "@inline"][@@merlin.hide ] + include struct let unit_two = unit_one + let _ = unit_two end[@@ocaml.doc "@inline"][@@merlin.hide ] + +As expected, there is a let _ = unit_two but nothing for unit_one. If we turn off +the unused-code-warnings flag, there will be one for both: + + $ ./driver.exe -unused-code-warnings=false -impl alias_warn.ml + type t = int[@@deriving alias_warn] + include struct let _ = fun (_ : t) -> () end[@@ocaml.doc "@inline"][@@merlin.hide + ] + include struct let unit_one = () + let _ = unit_one end[@@ocaml.doc "@inline"][@@merlin.hide ] + include struct let unit_two = unit_one + let _ = unit_two end[@@ocaml.doc "@inline"][@@merlin.hide ] + +Same goes for .mli: + + $ cat > alias_warn.mli << EOF + > type t = int [@@deriving alias_warn] + > EOF + + $ ./driver.exe -unused-code-warnings=true -intf alias_warn.mli + type t = int[@@deriving alias_warn] + include sig val unit_one : unit end[@@ocaml.doc "@inline"][@@merlin.hide ] + include sig [@@@ocaml.warning "-32"] val unit_two : unit end[@@ocaml.doc + "@inline"] + [@@merlin.hide ] + + $ ./driver.exe -unused-code-warnings=false -intf alias_warn.mli + type t = int[@@deriving alias_warn] + include sig [@@@ocaml.warning "-32"] val unit_one : unit end[@@ocaml.doc + "@inline"] + [@@merlin.hide ] + include sig [@@@ocaml.warning "-32"] val unit_two : unit end[@@ocaml.doc + "@inline"] + [@@merlin.hide ]