-
Notifications
You must be signed in to change notification settings - Fork 188
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
[BUG] ppx_expect output is being included in built javascript files #1625
Comments
Js_of_ocaml does a fine job of dead-code elimination, but because expect-tests are enabled or disabled vis environment variable, you need to tell jsoo that the expect-test env var is always empty. I forget which var it is, and I also forget how to configure dune to do this, but it does work! |
In general, you want this code to survive JS code generation so that you can run test in JavaScript. You should be able to drop the tests with the following js_of_ocaml compile flags. This allows js_of_ocaml to perform static evaluation of the following code
|
Thanks. Unfortunately I am still seeing the test strings in the file even after adding
to my dune config |
You also need to build with |
Or you can set |
I was using --profile release as well |
I can't reproduce. I see the expect_test being dropped when use what has been mentioned in the issue. Can you share some code ? if you're using dune, you should probably try to pass |
Let me update to 5.7.0 and then see if I can give you a reproducer |
The code base I am working with is https://github.com/stan-dev/stanc3. I put together a gist with my dune config, the output of dune Here is a string which only appears in the source code as part of the expect tests:
$ grep "(RequireAllCondition (Exact stan::is_vt_not_complex) (TemplateType T0__1__))" ./stancjs.bc.js -c
1 |
It could be a regression in ppx_inline_test or ppx_expect v0.17.0 |
Could you try to downgrade to v0.16.0 (assuming you are indeed using v0.17.0) |
I am using 0.16.0. Here's my Details
|
@TyOverby, it seems that the code structure generated by ppx_expect defeat the logic in place inside ppx_inline_test to allow deadcode of tests. In particular, the call to |
@WardBrian, you might want to open the issue against ppx_expect instead |
@hhugo thanks for the digging! Unfortunately updating the ppx dependencies is harder for us than JSOO, so I may be stuck with the broken behavior for a while, but I'll open a ticket on their repo |
xref the ppx_expect issue to here. I can provide some guidance on how to fix the ppx_expect code. |
fixed in janestreet/ppx_expect#56 |
For the record, you need at least two expect tests to reproduce the issue otherwise, inlining of the functor happens and the code is properly DCE |
Ah, that explains some of the difficulties reproducing. Thanks for getting a fix in before I even got around to writing a new issue! |
Describe the bug
I have tests in my code using ppx_expect. The string of these tests can be found in the generated
.bc.js
file from JSOOExpected behavior
That this code would not survive JS code generation.
Versions
5.5.2
The text was updated successfully, but these errors were encountered: