From 65ad057f771b7057971f2fded9527cea2e564e02 Mon Sep 17 00:00:00 2001
From: Nathan Rebours <nathan@cryptosense.com>
Date: Tue, 12 Mar 2019 10:54:47 +0100
Subject: [PATCH] Add error test infrastructure

---
 CONTRIBUTING.md                       | 46 +++++++++++++++++++++++++++
 test/deriver/errors/dune              | 34 ++++++++++++++++++++
 test/deriver/errors/dune.inc          |  0
 test/deriver/errors/gen_dune_rules.ml | 44 +++++++++++++++++++++++++
 test/deriver/errors/pp.ml             |  1 +
 5 files changed, 125 insertions(+)
 create mode 100644 CONTRIBUTING.md
 create mode 100644 test/deriver/errors/dune
 create mode 100644 test/deriver/errors/dune.inc
 create mode 100644 test/deriver/errors/gen_dune_rules.ml
 create mode 100644 test/deriver/errors/pp.ml

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 0000000..8a447d3
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,46 @@
+## Tests
+
+`ppx_factory` uses two layers of tests. Adding tests along with your contribution is definitely
+encouraged and appreciated!
+
+### Unit tests
+
+You can find unit tests for simple library functions in `test/lib`. They are written using OUnit.
+There is one test file per tested library module and the entire test suite is defined and run in
+`test_ppx_factory_lib.ml`.
+
+We should try testing stuff there as much as possible, especially small helper functions that don't
+produce chunks of AST.
+
+Each function has its own test suite under `test_<function_name>` and they are all grouped together
+under a `suite` value at the test module's top level.
+
+We try to mimic the module structure of the project in the tests. That means that if you want to add
+tests for a function from a submodule B from a module A, it should sit under a `Test_a.B` module
+which should have its own `suite`.
+
+### Deriver tests
+
+These are the main tests for `ppx_factory`. We use them to ensure the generated code is what we
+expect it to be.
+
+They are run by comparing the output of `ppx_factory` as a standalone binary against `.expected`
+files, both for successful and unsuccessful deriving.
+
+The successful cases are splitted between `test/deriver/test_default.ml` and
+`test/deriver/test_factory.ml` depending on which part of the plugin they correspond.  You can update
+those with new test cases and then run `dune runtest`. It will show the diff with the generated code
+for your new case, if you're happy with the result you can update the expected results by running
+`dune promote`.
+
+We also test the PPX errors to make sure they are properly triggered and located. There's one file
+per error case tested in `test/deriver/errors`. If you want to add a test case there you should:
+1. run `touch test/deriver/errors/factory_<test_case_name>.{ml,expected}` to add the new empty test
+   cases. Use `factory_` or `default_` as a prefix depending on which of the two derivers they
+   correspond to.
+2. run `dune runtest --auto-promote` to update the dune file with the rules for your new test case.
+3. update the `.ml` file with the erronous use of `ppx_factory`, you can use one of the other test
+   cases as an example. If your error is already produced correctly, merlin should highlight it in
+   your code.
+4. run `dune runtest` to see what the error is.
+5. if you're happy with the result, run `dune promote` to update the `.expected` file.
diff --git a/test/deriver/errors/dune b/test/deriver/errors/dune
new file mode 100644
index 0000000..b8e0aa1
--- /dev/null
+++ b/test/deriver/errors/dune
@@ -0,0 +1,34 @@
+(executable
+  (name pp)
+  (modules pp)
+  (libraries
+    ppx_factory
+    ppxlib
+  )
+)
+
+(include dune.inc)
+
+(executable
+  (name gen_dune_rules)
+  (modules gen_dune_rules)
+)
+
+(rule
+  (targets dune.inc.gen)
+  (deps
+    (:gen gen_dune_rules.exe)
+    (source_tree .)
+  )
+  (action
+    (with-stdout-to
+      %{targets}
+      (run %{gen})
+    ) 
+  )
+)
+
+(alias
+  (name runtest)
+  (action (diff dune.inc dune.inc.gen))
+)
diff --git a/test/deriver/errors/dune.inc b/test/deriver/errors/dune.inc
new file mode 100644
index 0000000..e69de29
diff --git a/test/deriver/errors/gen_dune_rules.ml b/test/deriver/errors/gen_dune_rules.ml
new file mode 100644
index 0000000..80ac39e
--- /dev/null
+++ b/test/deriver/errors/gen_dune_rules.ml
@@ -0,0 +1,44 @@
+let output_stanzas filename =
+  let base = Filename.remove_extension filename in
+  Printf.printf
+    {|
+(library
+  (name %s)
+  (modules %s)
+  (preprocess (pps ppx_factory))
+)
+
+(rule
+  (targets %s.actual)
+  (deps (:pp pp.exe) (:input %s.ml))
+  (action
+    (with-stderr-to
+      %%{targets}
+      (bash "./%%{pp} -no-color --impl %%{input} || true")
+    )
+  )
+)
+
+(alias
+  (name runtest)
+  (action (diff %s.expected %s.actual))
+)
+|}
+    base
+    base
+    base
+    base
+    base
+    base
+
+let is_error_test = function
+  | "pp.ml" -> false
+  | "gen_dune_rules.ml" -> false
+  | filename -> Filename.check_suffix filename ".ml"
+
+let () =
+  Sys.readdir "."
+  |> Array.to_list
+  |> List.sort String.compare
+  |> List.filter is_error_test
+  |> List.iter output_stanzas
diff --git a/test/deriver/errors/pp.ml b/test/deriver/errors/pp.ml
new file mode 100644
index 0000000..7526c2d
--- /dev/null
+++ b/test/deriver/errors/pp.ml
@@ -0,0 +1 @@
+Ppxlib.Driver.standalone ();