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

Ability to mark certain parts of code as covered in order to achieve 100% coverage. #63

Open
AndreyKozlov1984 opened this issue May 3, 2017 · 3 comments

Comments

@AndreyKozlov1984
Copy link

First of all, this looks like a great tool. I still find that, for example, when I call
const result: MyType = await response.json() on an isomorhic-fetch I get that part of code uncovered, whatever I do. The other place is a default (action: empty) guard in a react-redux reducer which is also reported as an uncovered code.

I want to try to get a 100% coverage, if that is possible at all, and thus would like to mark "unsafe" places somehow, the first idea is to analyze the generated json file, extract "uncovered_locs" together with a source code for those chunks and get rid of those pieces of code which I explicitly specify. The other option would be to somehow annotate those places directly, although no ideas how, may be with a special comment like
const result: MyType = /*UnsafeStart*/ response.json() /*UnsafeEnd*/

@ryan953
Copy link
Contributor

ryan953 commented May 5, 2017

Hey @ZeusTheTrueGod, marking unsafe sections of code is a really interesting idea. I can remember similar situations that I've encountered where I know some statements are wonky and that flow doesn't like it, but I just want to make it shut up so I can continue.

I'm not sure modifying this tool is the right place to make that happen though. Let me explain why.

Like you noted, currently flow-coverage-report calls out to the flow binary to get the coverage data with flow coverage --json. That coverage data won't include these "Unsafe" comments (or, if not comments, however we implement it). So flow-coverage-report would need to parse and the source directly to find those comments. This is possible, a very-large project, but possible. But the drawback is that your editor and any other flow related tools or linters you might use will not be aware of the "Unsafe" section. See coverage in Nuclide as one example.

So I'm thinking, this should probably be reported and implemented directly in facebook/flow, so all downstream tools, like flow-coverage-report and Nuclide benefit.

In the mean time, I have some workarounds to suggest!

  1. Maybe it's possible to extract the unsafe stuff into it's own file and then use the -x flag to exclude it from the report. If you name the file something like myFunc-unsafeTypes.js the intent would be clear to the next person too.
  2. Share some more code or a reduced test case. Sometimes using mixed or any is the right thing to do, with a little more context I might be able to help here.
  3. If you always expect the same variables to cause errors it might be possible to create a wrapper script that filters the json before handing it back to flow-coverage-report. You can use the -f flag to point to your script instead of at flow directly. Example: flow-coverage-report -f /bin/filter-some-json. This can get you there right away without being blocked on anyone else.

@AndreyKozlov1984
Copy link
Author

@ryan953 I had similar thoughts, thanks for providing the proper answer.
The ability to use the -f flag seems to be a great fix, because this way I'm able to make a custom wrapper for calling flow first and updating the generated json next in order to extract the 'Unsafe' comments. I see that 100% flow coverage is not an easy task in a real project, still your tool with an -f flag could help a lot.
My guess is that if somehow I write that wrapper, may be it would be possible to put that documentation here.

@rpl
Copy link
Owner

rpl commented May 5, 2017

@ZeusTheTrueGod Yeah, I totally agree with @ryan953, this feature would be very nice (maybe also paired with an eslint rule that prevent has to abuse of it :-)), but it would be better if flow is the place where the marked expressions are filtered out, and in the meantime the using the -f flag as a workaround sounds like a very good approach.

also, I would be more than happy to add some documentation about such wrapper once we have figured out the details 👍

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

3 participants