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

CLI tests: use dune to run ocamlformat #1157

Merged
merged 1 commit into from
Nov 22, 2019

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Nov 19, 2019

(partial revert of #1126, as discussed in #1147)

Test generators are a bit heavy, but running processes through ocaml is a bit limited when we want to set a working directory or environment.

So, let's use a middle ground solution: run ocamlformat through dune, but write the rules by hand.

@Julow
Copy link
Collaborator

Julow commented Nov 19, 2019

Why are test generators a bit heavy ?

@gpetiot gpetiot changed the title cli tests: use dune to run ocamlformat CLI tests: use dune to run ocamlformat Nov 20, 2019
@gpetiot
Copy link
Collaborator

gpetiot commented Nov 20, 2019

For me personally it is easier to maintain OCaml code than dune code but I don't have any other argument against writing rules manually, as long as we don't add new ones too often.

@emillon emillon force-pushed the partial-revert-1126 branch from ab008d4 to e3dc941 Compare November 22, 2019 09:50
@emillon
Copy link
Collaborator Author

emillon commented Nov 22, 2019

As discussed offline with @Julow, the main disadvantage of this approach is that adding new tests involve copy/pasting, so it's possible that we'll forget to add the runtest-diff part for one test someday.

Two things can make this easier in the future:

  • a generator whose input data is in a .ml file (not scattered across several files as was the case before)
  • some of these e2e tests will become less important as we add ways to test this in a more focused way.

Test generators are a bit heavy, but running processes through ocaml is
a bit limited when we want to set a working directory or environment.

So, let's use a middle ground solution: run ocamlformat through dune,
but write the rules by hand.
@emillon emillon force-pushed the partial-revert-1126 branch from e3dc941 to 1e6cca2 Compare November 22, 2019 12:36
@emillon emillon merged commit 3f2dddb into ocaml-ppx:master Nov 22, 2019
@emillon emillon deleted the partial-revert-1126 branch November 22, 2019 12:45
bogdan2412 pushed a commit to bogdan2412/ocamlformat that referenced this pull request Mar 28, 2020
Test generators are a bit heavy, but running processes through ocaml is
a bit limited when we want to set a working directory or environment.

So, let's use a middle ground solution: run ocamlformat through dune,
but write the rules by hand.
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.

4 participants