Skip to content

Commit

Permalink
Add comprehensive testing of the janestreet profile (#72)
Browse files Browse the repository at this point in the history
* add [*.js-ref] support to testing script

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* add [*.js-ref] tests throughout the test suite

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* delete empty [*.js-err]s

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* turn off local configs when running [*.js-ref]

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* ignore [*.opts] when testing [*.js-ref]

we don't internally permit command-line configuration, so we're mostly
interested in the default behavior of the janestreet profile

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* revert unneeded changes to [*.opts]

when the [*.js-ref] tests obeyed [.ocamlformat] and [*.opts], we used
[*.opts] to increase the default low [--max-iters]

now that we ignore [.ocamlformat] from [*.js-ref] tests, we don't need
to change [*.opts]

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* enforce comprehensive [*.js-ref] testing

we check that every non-js test either has a corresponding js test, or
an explanation of why it shouldn't be stressed under the janestreet
profile

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* remove most [--profile=janestreet] in [*.opts]

also merge [source.ml] and [js_source.ml]

one [*.opts] with [--profile=janestreet] remains because it is for a
file that is currently broken in the ocamlformat profile

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* run dune fmt

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* disable comprehensiveness tests on win

it suffices for these tests to run during local development on linux
and as part of the github ci

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* only generate js-coverage rules that will fail

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

---------

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
  • Loading branch information
dvulakh authored Jun 7, 2024
1 parent 3a23ed2 commit 67727be
Show file tree
Hide file tree
Showing 364 changed files with 41,217 additions and 20,284 deletions.
8,708 changes: 7,161 additions & 1,547 deletions test/passing/dune.inc

Large diffs are not rendered by default.

98 changes: 72 additions & 26 deletions test/passing/gen/gen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ let dep fname = spf "%%{dep:%s}" fname

type setup =
{ mutable has_ref: bool
; mutable has_js_ref: bool
; mutable has_why_no_js: bool
; mutable has_opts: bool
; mutable has_ocp: bool
; mutable ocp_opts: string list
Expand Down Expand Up @@ -41,6 +43,8 @@ let read_file file =
let add_test ?base_file map src_test_name =
let s =
{ has_ref= false
; has_js_ref= false
; has_why_no_js= false
; has_opts= false
; has_ocp= false
; ocp_opts= []
Expand Down Expand Up @@ -75,12 +79,14 @@ let register_file tests fname =
| ["output"] | ["ocp"; "output"] -> ()
| ["opts"] -> setup.has_opts <- true
| ["ref"] -> setup.has_ref <- true
| ["js-ref"] -> setup.has_js_ref <- true
| ["why-no-js"] -> setup.has_why_no_js <- true
| ["ocp"] -> setup.has_ocp <- true
| ["ocp-opts"] -> setup.ocp_opts <- read_lines fname
| ["deps"] -> setup.extra_deps <- read_lines fname
| ["should-fail"] -> setup.should_fail <- true
| ["enabled-if"] -> setup.enabled_if <- Some (read_file fname)
| ["err"] -> ()
| [("err" | "js-err")] -> ()
| _ -> invalid_arg fname )
| _ -> ()

Expand All @@ -93,6 +99,54 @@ let cmd should_fail args =
(run %s))|} cmd_string
else spf {|(run %s)|} cmd_string

let one_styling_test ~extra_deps ~enabled_if_line ~test_name ~base_test_name
~should_fail ~opts ~output_name ~extra_suffix =
Printf.sprintf
{|
(rule
(deps tests/.ocamlformat %s)%s
(package ocamlformat)
(action
(with-stdout-to %s.%sstdout
(with-stderr-to %s.%sstderr
%s))))

(rule
(alias runtest)%s
(package ocamlformat)
(action (diff %s %s.%sstdout)))

(rule
(alias runtest)%s
(package ocamlformat)
(action (diff tests/%s.%serr %s.%sstderr)))
|}
extra_deps enabled_if_line test_name extra_suffix test_name extra_suffix
(cmd should_fail (["%{bin:ocamlformat}"] @ opts @ [dep base_test_name]))
enabled_if_line output_name test_name extra_suffix enabled_if_line
test_name extra_suffix test_name extra_suffix

let one_js_coverage_test ~test_name ~has_js_ref ~has_why_no_js =
let (err_msg : _ format option) =
match (has_js_ref, has_why_no_js) with
| true, true -> Some "%s has both a [.js-ref] and a [.why-no-js]!"
| false, false -> Some "%s has neither a [.js-ref], nor a [.why-no-js]!"
| true, false | false, true -> None
in
match err_msg with
| Some err_msg ->
let err_msg = Printf.sprintf err_msg test_name in
Printf.sprintf
{|
(rule
(alias runtest)
(enabled_if (<> %%{os_type} Win32))
(package ocamlformat)
(action (system "echo '%s'; exit 1")))
|}
err_msg
| None -> ""

let emit_test test_name setup =
let opts =
"--margin-check"
Expand All @@ -103,7 +157,7 @@ let emit_test test_name setup =
let ref_name =
"tests/" ^ if setup.has_ref then test_name ^ ".ref" else test_name
in
let err_name = "tests/" ^ test_name ^ ".err" in
let js_ref_name = "tests/" ^ test_name ^ ".js-ref" in
let base_test_name =
"tests/" ^ match setup.base_file with Some n -> n | None -> test_name
in
Expand All @@ -114,30 +168,22 @@ let emit_test test_name setup =
| Some clause -> spf "\n (enabled_if %s)" clause
in
let output_fname = test_name ^ ".stdout" in
Printf.printf
{|
(rule
(deps tests/.ocamlformat %s)%s
(package ocamlformat)
(action
(with-stdout-to %s
(with-stderr-to %s.stderr
%s))))

(rule
(alias runtest)%s
(package ocamlformat)
(action (diff %s %s.stdout)))

(rule
(alias runtest)%s
(package ocamlformat)
(action (diff %s %s.stderr)))
|}
extra_deps enabled_if_line output_fname test_name
(cmd setup.should_fail
(["%{bin:ocamlformat}"] @ opts @ [dep base_test_name]) )
enabled_if_line ref_name test_name enabled_if_line err_name test_name ;
one_styling_test ~extra_deps ~enabled_if_line ~test_name ~base_test_name
~should_fail:setup.should_fail ~opts ~output_name:ref_name
~extra_suffix:""
|> print_string ;
if setup.has_js_ref then
one_styling_test ~extra_deps ~enabled_if_line ~test_name ~base_test_name
~should_fail:setup.should_fail
~opts:
[ "--profile=janestreet"
; "--enable-outside-detected-project"
; "--disable-conf-files" ]
~output_name:js_ref_name ~extra_suffix:"js-"
|> print_string ;
one_js_coverage_test ~test_name ~has_js_ref:setup.has_js_ref
~has_why_no_js:setup.has_why_no_js
|> print_string ;
if setup.has_ocp then
let ocp_cmd =
"%{bin:ocp-indent}" :: (setup.ocp_opts @ [dep output_fname])
Expand Down
3 changes: 3 additions & 0 deletions test/passing/tests/align_infix.ml.js-ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let sum_of_squares num =
num + 1 |> List.range 0 |> List.map ~f:square |> List.fold_left ~init:0 ~f:( + )
;;
14 changes: 14 additions & 0 deletions test/passing/tests/alignment.ml.js-ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
let file_contents = [] @ [ foo ] @ [ bar ]

let _ =
match s.src with
| None -> [ zz ] + 2
| Some s ->
[ Variable (s_src, OpamFormat.make_string (OpamFilename.to_string s)); yy ];
foo
| Some s -> { fww = s_src, OpamFormat.make_string (OpamFilename.to_string s); gdd = yy }
;;

let _ = [ x; y ] @ z
let _ = [ x; y ] @ z
let _ = [ x; y ] @ z
98 changes: 98 additions & 0 deletions test/passing/tests/apply.ml.js-ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
let _ = List.map ~f:(( + ) (M.f x))
let id x = x
let plus a ?(b = 0) c = a + b + c;;

id (plus 1) ~b:1;;

(* The version above does not type-check, while the version below does
type-check, and should not be formatted to the above. See
https://caml.inria.fr/mantis/view.php?id=7832 for explanation on the
type-checking (and dynamic semantics) distinction. *)

(id (plus 1)) ~b:1

let ( !!! ) a ~b = a + b
let _ = ( !!! ) a b
let _ = ( !!! ) ~b
let _ = !!!!a b d
let _ = ( + ) a b c d

let cartesian_product l1 l2 =
List.concat (l1 |> List.map (fun v1 -> l2 |> List.map (fun v2 -> v1, v2)))
;;

let cartesian_product' long_list_one long_list_two =
List.concat
(long_list_one |> List.map (fun v1 -> long_list_two |> List.map (fun v2 -> v1, v2)))
;;

let whatever a_function_name long_list_one some_other_thing =
List.map
(fun long_list_one_elt ->
do_something_with_a_function_and_some_things
a_function_name
long_list_one_elt
some_other_thing)
long_list_one
;;

let whatever_labelled a_function_name long_list_one some_other_thing =
ListLabels.map long_list_one ~f:(fun long_list_one_elt ->
do_something_with_a_function_and_some_things
a_function_name
long_list_one_elt
some_other_thing)
;;

[@@@ocamlformat "indicate-multiline-delimiters=closing-on-separate-line"]

let cartesian_product' long_list_one long_list_two =
List.concat
(long_list_one |> List.map (fun v1 -> long_list_two |> List.map (fun v2 -> v1, v2)))
;;

let whatever a_function_name long_list_one some_other_thing =
List.map
(fun long_list_one_elt ->
do_something_with_a_function_and_some_things
a_function_name
long_list_one_elt
some_other_thing
)
long_list_one
;;

let whatever_labelled a_function_name long_list_one some_other_thing =
ListLabels.map long_list_one ~f:(fun long_list_one_elt ->
do_something_with_a_function_and_some_things
a_function_name
long_list_one_elt
some_other_thing
)
;;

(a - b) ();;
((a - b) [@foo]) ()

let _ = M.(loooooooooooooooooooooong + loooooooooooooooooong)

let _ =
M.(
loooooooooooooooooooooong
+ loooooooooooooooooong
+ llllllllllloooooooooooooooooonnnnnnnnnnnnnggggggggggg
)
;;

let _ =
i'm_a_function
loooooooooooong
(loooooooooooong
looooooooooooooong
loooooooooooooong
[ loooooooooong; loooooooooooong; loooooooooooooooooooooong ]
)
;;

let f (x :: y) = x
let f (* xx *) ((* aa *) x (* bb *) :: (* cc *) y (* dd *)) (* yy *) = x
2 changes: 2 additions & 0 deletions test/passing/tests/apply_functor.ml.js-ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module _ = F (functor (X : T) -> X)
module _ = F (functor (X____________________________ : T) -> X____________________________)
2 changes: 2 additions & 0 deletions test/passing/tests/args_grouped-conventional.ml.why-no-js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
we can't test behavior in the "conventional" profile from the "janestreet"
profile
122 changes: 122 additions & 0 deletions test/passing/tests/args_grouped.ml.js-ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
let nullsafe_optimistic_third_party_params_in_non_strict =
CLOpt.mk_bool
~long:"nullsafe-optimistic-third-party-params-in-non-strict"
(* Turned on for compatibility reasons. Historically this is because
there was no actionable way to change third party annotations. Now
that we have such a support, this behavior should be reconsidered,
provided our tooling and error reporting is friendly enough to be
smoothly used by developers. *)
~default:true
"Nullsafe: in this mode we treat non annotated third party method params as if they \
were annotated as nullable."
;;

let test_file_renamings_from_json =
let create_test test_input expected_output _ =
let test_output input =
DifferentialFilters.FileRenamings.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.from_json
input
in
foo
in
fooooooooooooooo
;;

let eval location exp0 astate =
let rec eval exp astate =
match (exp : Exp.t) with
| Var id ->
Ok (eval_var (* error in case of missing history? *) [] (Var.of_id id) astate)
| Lvar pvar ->
Ok
(eval_var
[ ValueHistory.VariableAccessed (pvar, location) ]
(Var.of_pvar pvar)
astate)
| Lfield (exp', field, _) -> goooooooo
in
fooooooooooooooooooooo
;;

let declare_locals_and_ret tenv pdesc (prop_ : Prop.normal Prop.t) =
let foooooooooooooo =
BiabductionConfig.run_in_re_execution_mode
(* no footprint vars for locals *)
sigma_locals_and_ret
()
in
fooooooooooooooooooooooooooo
;;

let bottom_up fooooooooooo =
let empty = Int.equal 0 !scheduled && Queue.is_empty pending in
if empty
then (
remaining := 0;
L.progress
"Finished call graph scheduling, %d procs remaining (in, or reaching, cycles).@."
(CallGraph.n_procs syntactic_call_graph);
if Config.debug_level_analysis > 0
then CallGraph.to_dotty syntactic_call_graph "cycles.dot";
foooooooooooooooooo)
else fooooooooooooooooo
;;

let test_file_renamings_from_json =
let fooooooooooooo =
match expected_output with
| Return exp ->
assert_equal
~pp_diff
~cmp:
DifferentialFilters.FileRenamings.VISIBLE_FOR_TESTING_DO_NOT_USE_DIRECTLY.equal
exp
(test_output test_input)
| Raise exc -> assert_raises exc (fun () -> test_output test_input)
in
foooooooooooooooo
;;

let gen_with_record_deps ~expand t resolved_forms ~dep_kind =
let foooooooooooooooooooooo =
expand
(* we keep the dir constant here to replicate the old behavior of:
(chdir foo %{exe:bar}). This should lookup ./bar rather than
./foo/bar *)
resolved_forms
~dir:t.dir
~dep_kind
~expand_var:t.expand_var
in
{ t with expand_var }
;;

let f =
very_long_function_name
~very_long_variable_name:(very_long expression)
(* this is a
multiple-line-spanning
comment *)
~y
;;

let eradicate_meta_class_is_nullsafe =
register
~id:"ERADICATE_META_CLASS_IS_NULLSAFE"
~hum:"Class is marked @Nullsafe and has 0 issues"
(* Should be enabled for special integrations *)
~enabled:false
Info
Eradicate (* TODO *)
~user_documentation:""
;;

let eradicate_meta_class_is_nullsafe =
register
~id:
"ERADICATE_META_CLASS_IS_NULLSAFE" (* Should be enabled for special integrations *)
~hum:"Class is marked @Nullsafe and has 0 issues"
(* Should be enabled for special integrations *)
~enabled:false
Info
;;
Loading

0 comments on commit 67727be

Please sign in to comment.