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

more flexibility for compiling binaries from inline tests #766

Closed
c-cube opened this issue May 11, 2018 · 11 comments
Closed

more flexibility for compiling binaries from inline tests #766

c-cube opened this issue May 11, 2018 · 11 comments

Comments

@c-cube
Copy link

c-cube commented May 11, 2018

I like the inline test generator feature (unsurprisingly) but for one of my projects (containers) I need the generated test to be compiled with -no-labels. It would probably be useful to also be able to specify other flags, warnings, etc.

Would it make sense that a subsection of the (inline_tests (…)) portion of a library has the same stanzas as any executable?

@ghost
Copy link

ghost commented May 14, 2018

Yh, that seems reasonable. Not all fields make sense, but we can import the relevant ones.

@lubegasimon
Copy link
Collaborator

@rgrinberg, @jeremiedimino, I would like to take on this, hope it's still alive ?

@ghost
Copy link

ghost commented Jul 21, 2020

@lubegasimon, if that would be useful for users, it's definitely still alive :) Would you have a use for this feature yourself?

@c-cube do you still need this?

@c-cube
Copy link
Author

c-cube commented Jul 21, 2020

I could use it in containers, which still does qtest stuff by hand. That's my one use case, and it's not particularly urgent.

@lubegasimon
Copy link
Collaborator

lubegasimon commented Jul 21, 2020

@lubegasimon, if that would be useful for users, it's definitely still alive :) Would you have a use for this feature yourself?

@jeremiedimino, am not having any need for this feature as of now, but if someone may need it today or not, then I'll be happy to help, and for the better tomorrow.

@ghost
Copy link

ghost commented Jul 27, 2020

Alright, first we need to decide what the new field will be called. There is already a flags field in inline_tests, however it is for flags passed to the test runner binary rather than the compiler. I propose to call the new field compile_flags.

Here are some pointers to add this feature:

  • parsing the new field:
    • add a field compile_flags : Ocaml_flags.Spec.t to the Info.t record in the functor application include Sub_system.Register_end_point (struct in src/dune/inline_tests.ml
    • in the same module, add and+ compile_flags = Ocaml_flags.Spec.decode in the decode function
  • interpreting the flags, in the gen_rules function in the same file:
    • the Ocaml_flags.Spec.t is a specification, not an actual list of flags. The first step is to convert this specification into a actual list of flags. This is done via the function Super_context.ocaml_flags. This function takes a Buildable.t which we don't have at this point. However, looking at the implementation of Super_context.ocaml_flags, it seems that we could easily change it to take a Ocaml_flags.Spec.t instead. So the first step is to do that
    • Once we have an Ocaml_flags.t value, we need to pass it to the call to Compilation_context.create in gen_rules via the ~flags argument. At the moment, we pass a hard-coded list of flags. We can use Ocaml_flags.append_common to append this hard-coded list of flags to the one we just obtained

Finally, the last and most important step is to update the documentation in doc/tests.rst. The HACKING.md file explains how to build the manual (in the section Documentation).

@shonfeder
Copy link
Collaborator

Also, adding tests :)

@lubegasimon
Copy link
Collaborator

Also, adding tests :)

nice point @shonfeder. Thank you.

@lubegasimon
Copy link
Collaborator

lubegasimon commented Aug 5, 2020

* add a field `compile_flags : Ocaml_flags.Spec.t` to the `Info.t`

@jeremiedimino, it's not so clear as to why compile_flags should be of type Ocaml_flags.Spec.t and not Ordered_set_lang.Unexpanded.t as it is the case with flags field, because when I use Ocaml_flags.Spec.t and run the tests, compile flags field is unknown.

@ghost
Copy link

ghost commented Aug 5, 2020

Indeed. I forgot that Ocaml_flags.Spec.t is a set of three flags:

  • flags
  • ocamlc_flags
  • ocamlopt_flags

So we can't really change the name here. In this case we can indeed use Ordered_set_lang.Unexpanded.t for compile_flags.

lubegasimon added a commit to lubegasimon/dune that referenced this issue Aug 5, 2020
… tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Aug 6, 2020
… tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Aug 16, 2020
… tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Aug 16, 2020
… tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Aug 18, 2020
… tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Aug 26, 2020
…ne tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
shonfeder pushed a commit to shonfeder/dune that referenced this issue Aug 31, 2020
…ne tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Sep 4, 2020
…ne tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
lubegasimon added a commit to lubegasimon/dune that referenced this issue Sep 10, 2020
…ne tests

Signed-off-by: lubegasimon <lubegasimon73@gmail.com>
@lubegasimon lubegasimon self-assigned this Sep 11, 2020
@lubegasimon
Copy link
Collaborator

Fixed in #3747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants