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

Help with BuckleScript and bs-jest #307

Closed
wyze opened this issue Apr 11, 2020 · 14 comments
Closed

Help with BuckleScript and bs-jest #307

wyze opened this issue Apr 11, 2020 · 14 comments
Milestone

Comments

@wyze
Copy link

wyze commented Apr 11, 2020

Hello and great project!

This is more of a help request than an issue, but I am trying to get coverage report to work when running tests via bs-jest.

Nothing is outputted when following the BuckleScript documentation. However if I add the Bisect.Runtime.write_coverage_data_on_exit(); line inside the module (not tests), and build and run the file node file.bs.js I do get output, but the coverage is at 0%.

Any help is greatly appreciated, and you can checkout the project I am trying it on here: https://github.com/wyze/bs-react-testing-library

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

@wyze What commands are you running to test with coverage? Can you show them? Are you including BISECT_ENABLE=yes? (this is a temporary workaround until rescript-lang/rescript#3761).

@wyze
Copy link
Author

wyze commented Apr 12, 2020

Thanks for taking a look. I just pushed a branch of me attempting to get it to work. https://github.com/wyze/bs-react-testing-library/tree/bisect_ppx

Yes, I added that env variable before yarn build. On that branch you can run yarn test or npm run test and it should clean and rebuild and run the tests.

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

Could you please give the exact commands you enter at the terminal to try to get coverage on that branch?

@wyze
Copy link
Author

wyze commented Apr 12, 2020

I use yarn test.

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

I patched like this to get a coverage report:

--- a/src/__tests__/ReactTestingLibrary_test.re
+++ b/src/__tests__/ReactTestingLibrary_test.re
@@ -1,6 +1,6 @@
 open Jest;

-Bisect.Runtime.write_coverage_data_on_exit();
+afterAll(Bisect.Runtime.write_coverage_data);

 module Greeting = {
   [@react.component]

It seems that Jest isn't triggering process.on('exit') hooks. I'll add a note to the README about the workaround above, but I think it won't be suitable for projects that have multiple test files. Is there a better way to do this with Jest?

You may also want to add [@coverage exclude_file]; in your test file to exclude from the report, leaving only your code proper. And, don't forget to rm -f *.coverage before running the tests :)

@wyze
Copy link
Author

wyze commented Apr 12, 2020

Awesome! I should have tried those hooks! Yes I will add the things to .gitignore. Now I can switch my Reason projects to Codecov!

I don't mind doing a PR to update the readme and the bsb-starter-project with extra instructions for bs-jest.

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

I suppose one way to do this in projects with multiple test files would be to ask users to do

afterAll(() => {
  Bisect.Runtime.write_coverage_data();
  Bisect.Runtime.reset_counters();
});

in each test file. reset_counters is only exposed on native right now, so I'd need to patch Bisect. It's a pretty awkward method. If you (or anyone reading this issue in the future) know or find out how to add an exit hook globally to the whole Jest run, please let me know :)

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

Awesome! I should have tried those hooks! Yes I will add the things to .gitignore. Now I can switch my Reason projects to Codecov!

Great :) Regarding .gitignore, if that's in reference to rm -f *.coverage, what I meant is that you should delete the .coverage files from previous runs before each new run, otherwise the reporter will combine them all into one report, and this could show falsely inflated coverage. BTW nice coverage in your project :)

I don't mind doing a PR to update the readme and the bsb-starter-project with extra instructions for bs-jest.

I would be happy to merge such a PR :)

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

Perhaps rm -f *.coverage also belongs in the starter repo README...

@wyze
Copy link
Author

wyze commented Apr 12, 2020

It does look like there is this, https://jestjs.io/docs/en/configuration#globalteardown-string, but it is all JS so not sure if that would work for you.

@aantron
Copy link
Owner

aantron commented Apr 12, 2020

Thanks!

It looks complicated enough that I'll keep it as a note until someone genuinely needs Bisect to support that.

@wyze
Copy link
Author

wyze commented Apr 12, 2020

So, I went and tried the same setup in another project, but when running with my test:coverage command, it throws some errors and doesn't complete. If you have some more free time, it would be awesome for you to take a look!

CI: https://github.com/wyze/bs-jest-dom/pull/13/checks?check_run_id=581289039
PR: wyze/bs-jest-dom#13

@aantron aantron added this to the 2.3.1 milestone Apr 13, 2020
@aantron
Copy link
Owner

aantron commented Apr 13, 2020

So, I went and tried the same setup in another project, but when running with my test:coverage command, it throws some errors and doesn't complete.

That's almost certainly an issue with Bisect_ppx not respecting the special meaning of ## in BuckleScript. I opened #308 to fix that. I'll do it in the coming days, then release 2.3.1 with the fix.

aantron added a commit to aantron/bisect-starter-rescript that referenced this issue Apr 13, 2020
@aantron
Copy link
Owner

aantron commented Apr 13, 2020

I updated the README with instructions to use afterAll if using Jest, and also exposed reset_counters (as reset_coverage_data) in the BuckleScript Bisect_ppx runtime, in case Bisect_ppx needs to switch recommending that later.

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

No branches or pull requests

2 participants