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

No annotation, no coverage #102

Closed
wants to merge 1 commit into from
Closed

Conversation

dmnd
Copy link

@dmnd dmnd commented Jul 10, 2017

When calculating coverage for a project, I don't care about coverage in
files that don't have the @flow pragma. While it's nice that Flow
infers types anyway, since it's never going to report an error it
doesn't help me at all and is potentially misleading. For example, my
project is 75% covered with master, but only 34% covered with this
change.

This PR changes flow-coverage-report to ignore coverage in unannotated
files. This could be considered a breaking change or a bugfix - I'm not
sure. You might consider making this a configuration option.

Resolves #85

When calculating coverage for a project, I don't care about coverage in
files that don't have the `@flow` pragma. While it's nice that Flow
infers types anyway, since it's never going to report an error it
doesn't help me at all and is potentially misleading. For example, my
project is 75% covered with master, but only 34% covered with this
change.

This PR changes flow-coverage-report to ignore coverage in unannotated
files. This could be considered a breaking change or a bugfix - I'm not
sure. You might consider making this a configuration option.

Resolves rpl#85
@rpl
Copy link
Owner

rpl commented Jul 11, 2017

@dmnd Yeah, I definitely agree.

When @ryan953 contributed the @flow annotation detection, we were discussing which kind of behavior would be reasonable to be the default.

The following can be a reasonable behavior:

  • the command exit with a failure exit code if the report includes any file which doesn't have the @flow pragma
  • define a new command line option which automatically exclude any file that doesn't include the @flow pragma (also customizable using the configuration, like the other command line options)

@dmnd @ryan953 How that sounds to you?

@dmnd
Copy link
Author

dmnd commented Jul 11, 2017

That sounds pretty good to me @rpl 👍

@ryan953
Copy link
Contributor

ryan953 commented Jul 11, 2017

I was thinking that we'd want to add a flag in order to keep backwards compat for people, but either way sounds good. I would use the only-@flow files mode because it's easier to explain that "75% coverage of files with @flow, those other files don't count"

@dmnd
Copy link
Author

dmnd commented Jul 11, 2017

I'd use "no annotation, no coverage" mode because I have a long term goal of getting to 100% coverage everywhere.

@arjun-menon
Copy link

This would be very useful for me as well. Are there plans to merge this soon, @dmnd? It seems like the only thing blocking this is a broken test. I'd happy to fix/add the tests for this.

@dmnd
Copy link
Author

dmnd commented Oct 17, 2017

@arjun-menon I don't have time to make the changes @rpl suggested and fix the broken test. I've just been using my fork to measure Flow coverage in my project.

I'd really appreciate you taking this over and landing it! ❤️

@rpl
Copy link
Owner

rpl commented Feb 18, 2018

Closing in favor of #141 (which also include a new test case)

@rpl rpl closed this Feb 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to report coverage only on files marked with "// @flow"?
4 participants