Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customize how (test) is run using (action) #1248

Merged
merged 1 commit into from
Sep 10, 2018
Merged

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Sep 10, 2018

No description provided.

@emillon emillon requested a review from a user September 10, 2018 12:34
@emillon emillon requested a review from rgrinberg as a code owner September 10, 2018 12:34
@emillon emillon force-pushed the tests-stanza-action branch 2 times, most recently from f10eaea to ce267f5 Compare September 10, 2018 12:51
src/gen_rules.ml Outdated
| Some a -> a
| None -> Action.Unexpanded.Run (String_with_vars.make_text loc test_exe, [])
in
let extra_bindings = Pform.Map.singleton "test" (Pform.Var.Values [Value.String test_exe]) in
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value should be a Path so that it expands to the right path if the user does: (chdir %{project_root} (run %{test})). Additionally, it should resolve to the executable in the host build context in case of cross-compilation (the function to do that is Super_context.Action.map_exe).

@emillon emillon force-pushed the tests-stanza-action branch from ce267f5 to 729c1e1 Compare September 10, 2018 14:39
@emillon
Copy link
Collaborator Author

emillon commented Sep 10, 2018

I fixed this. I changed it so that the default action is (run %{test}), which takes case of this chdir and cross compilation issues for the default case as well.

src/gen_rules.ml Outdated
} in
match t.action with
| Some a -> a
| None -> Action.Unexpanded.Run (String_with_vars.virt_var __POS__ test_var_name, [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a fake loc here? Isn't it better to inherit the loc of the test name?

Closes #1195

Signed-off-by: Etienne Millon <me@emillon.org>
@emillon emillon force-pushed the tests-stanza-action branch from 729c1e1 to b838031 Compare September 10, 2018 15:03
@emillon
Copy link
Collaborator Author

emillon commented Sep 10, 2018

good idea! just added make_var to do that.

Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice change!

@emillon emillon merged commit a90c7e8 into master Sep 10, 2018
@emillon emillon deleted the tests-stanza-action branch September 10, 2018 16:03
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Sep 14, 2018
CHANGES:

- Ignore stderr output when trying to find out the number of jobs
  available (ocaml/dune#1118, fix ocaml/dune#1116, @diml)

- Fix error message when the source directory of `copy_files` does not exist.
  (ocaml/dune#1120, fix ocaml/dune#1099, @emillon)

- Highlight error locations in error messages (ocaml/dune#1121, @emillon)

- Display actual stanza when package is ambiguous (ocaml/dune#1126, fix ocaml/dune#1123, @emillon)

- Add `dune unstable-fmt` to format `dune` files. The interface and syntax are
  still subject to change, so use with caution. (ocaml/dune#1130, fix ocaml/dune#940, @emillon)

- Improve error message for `dune utop` without a library name (ocaml/dune#1154, fix
  ocaml/dune#1149, @emillon)

- Fix parsing `ocamllex` stanza in jbuild files (ocaml/dune#1150, @rgrinberg)

- Highlight multi-line errors (ocaml/dune#1131, @anuragsoni)

- Do no try to generate shared libraries when this is not supported by
  the OS (ocaml/dune#1165, fix ocaml/dune#1051, @diml)

- Fix `Flags.write_{sexp,lines}` in configurator by avoiding the use of
  `Stdune.Path` (ocaml/dune#1175, fix ocaml/dune#1161, @rgrinberg)

- Add support for `findlib.dynload`: when linking an executable using
  `findlib.dynload`, automatically record linked in libraries and
  findlib predicates (ocaml/dune#1172, @bobot)

- Add support for promoting a selected list of files (ocaml/dune#1192, @diml)

- Add an emacs mode providing helpers to promote correction files
  (ocaml/dune#1192, @diml)

- Improve message suggesting to remove parentheses (ocaml/dune#1196, fix ocaml/dune#1173, @emillon)

- Add `(wrapped (transition "..message.."))` as an option that will generate
  wrapped modules but keep unwrapped modules with a deprecation message to
  preserve compatibility. (ocaml/dune#1188, fix ocaml/dune#985, @rgrinberg)

- Fix the flags passed to the ppx rewriter when using `staged_pps` (ocaml/dune#1218, @diml)

- Add `(env var)` to add a dependency to an environment variable.
  (ocaml/dune#1186, @emillon)

- Add a simple version of a polling mode: `dune build -w` keeps
  running and restarts the build when something change on the
  filesystem (ocaml/dune#1140, @kodek16)

- Cleanup the way we detect the library search path. We no longer call
  `opam config var lib` in the default build context (ocaml/dune#1226, @diml)

- Make test stanzas honor the -p flag. (ocaml/dune#1236, fix ocaml/dune#1231, @emillon)

- Test stanzas take an optional (action) field to customize how they run (ocaml/dune#1248,
  ocaml/dune#1195, @emillon)

- Add support for private modules via the `private_modules` field (ocaml/dune#1241, fix
  ocaml/dune#427, @rgrinberg)

- Add support for passing arguments to the OCaml compiler via a
  response file when the list of arguments is too long (ocaml/dune#1256, @diml)

- Do not print diffs by default when running inside dune (ocaml/dune#1260, @diml)

- Interpret `$ dune build dir` as building the default alias in `dir`. (ocaml/dune#1259,
  @rgrinberg)

- Make the `dynlink` library available without findlib installed (ocaml/dune#1270, fix
  ocaml/dune#1264, @rgrinberg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants